Factor out UI modules unlock and pin (#97)

- Factor out unlock and pin modules
- Add unit tests for unlock and pin

ledger/src/ui/src/bolos_ux.c

@@ -52,16 +52,13 @@
 #include "attestation.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_LENGTH + 2];
-// Skip the prepended length of pin buffer
-#define G_PIN_BUFFER_PAYLOAD (G_pin_buffer + 1)
-// Unify pin buffer length
-#define G_PIN_BUFFER_LEN() strlen((const char *)G_PIN_BUFFER_PAYLOAD)
+unsigned char G_pin_buffer[PIN_BUFFER_LENGTH];
 
 #ifdef OS_IO_SEPROXYHAL
 
@@ -243,6 +240,9 @@ 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)
+
 // Operation being currently executed
 static unsigned char curr_cmd;
 
@@ -258,13 +258,12 @@ static void reset_if_starting(unsigned char 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.pin_buffer,
-                       sizeof(G_bolos_ux_context.pin_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);
     }
 }
 
@@ -321,11 +320,8 @@ static void sample_main(void) {
                     break;
                 case RSK_PIN_CMD: // Send pin_buffer
                     reset_if_starting(RSK_META_CMD_UIOP);
-                    pin = APDU_AT(2);
-                    if ((pin >= 0) && (pin <= PIN_LENGTH)) {
-                        G_pin_buffer[pin] = APDU_AT(3);
-                        G_pin_buffer[pin + 1] = 0;
-                    }
+                    init_pin_ctx(&pin_ctx, G_pin_buffer);
+                    tx = update_pin_buffer(rx, &pin_ctx);
                     THROW(APDU_OK);
                     break;
                 case RSK_IS_ONBOARD: // Wheter it's onboarded or not
@@ -347,8 +343,9 @@ static void sample_main(void) {
                     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(G_PIN_BUFFER_PAYLOAD)) {
+                    if (!is_pin_valid(&pin_ctx)) {
                         THROW(ERR_INVALID_PIN);
                     }
 #endif
@@ -392,14 +389,14 @@ static void sample_main(void) {
                                    sizeof(G_bolos_ux_context.words_buffer));
                     // Set PIN
                     os_perso_set_pin(
-                        0, G_PIN_BUFFER_PAYLOAD, G_PIN_BUFFER_LEN());
+                        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(G_PIN_BUFFER_PAYLOAD,
-                                                    G_PIN_BUFFER_LEN()));
+                                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
@@ -413,22 +410,13 @@ static void sample_main(void) {
                     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(G_PIN_BUFFER_PAYLOAD)) {
+                    if (!is_pin_valid(&pin_ctx)) {
                         THROW(ERR_INVALID_PIN);
                     }
 #endif
-                    // Set PIN
-                    os_perso_set_pin(
-                        0, G_PIN_BUFFER_PAYLOAD, G_PIN_BUFFER_LEN());
-                    // check PIN
-                    os_global_pin_invalidate();
-                    SET_APDU_AT(1, 2);
-                    SET_APDU_AT(2,
-                                os_global_pin_check(G_PIN_BUFFER_PAYLOAD,
-                                                    G_PIN_BUFFER_LEN()));
-                    tx = 3;
+                    tx = set_device_pin(rx, &pin_ctx);
                     // Clear pin buffer
                     explicit_bzero(G_pin_buffer, sizeof(G_pin_buffer));
                     THROW(APDU_OK);
@@ -462,18 +450,12 @@ static void sample_main(void) {
                     break;
                 case RSK_UNLOCK_CMD: // Unlock
                     reset_if_starting(RSK_META_CMD_UIOP);
-                    // RSK_UNLOCK_CMD does not send the prepended length,
-                    // so we kept this call backwards compatible, using
-                    // G_pin_buffer instead of G_PIN_BUFFER_PAYLOAD
-                    SET_APDU_AT(
-                        2,
-                        os_global_pin_check(
-                            G_pin_buffer, strlen((const char *)G_pin_buffer)));
-                    tx = 5;
-                    THROW(APDU_OK);
+                    init_pin_ctx(&pin_ctx, G_pin_buffer);
+                    tx = unlock(rx, &pin_ctx);
                     // The pin value will also be used in
                     // BOLOS_UX_CONSENT_APP_ADD command, so we can't wipe the
                     // pin buffer here
+                    THROW(APDU_OK);
                     break;
                 case RSK_END_CMD: // return to dashboard
                     reset_if_starting(RSK_END_CMD);
@@ -652,8 +634,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
-                os_global_pin_check(G_pin_buffer,
-                                    strlen((const char *)G_pin_buffer));
+                init_pin_ctx(&pin_ctx, G_pin_buffer);
+                os_global_pin_check(pin_ctx.pin_buffer,
+                                    strlen((const char *)pin_ctx.pin_buffer));
                 G_bolos_ux_context.exit_code = BOLOS_UX_OK;
                 explicit_bzero(G_pin_buffer, sizeof(G_pin_buffer));
                 break;

ledger/src/ui/src/bolos_ux.h

@@ -160,14 +160,10 @@ typedef struct bolos_ux_context {
         union {
             att_t attestation;
             sigaut_t sigaut;
+            pin_t pin;
         };
     };
 
-#define MAX_PIN_LENGTH 8
-#define MIN_PIN_LENGTH 4
-    char pin_buffer[MAX_PIN_LENGTH +
-                    1]; // length prepended for custom pin length
-
     // filled up during os_ux syscall when called by user or bolos.
     bolos_ux_params_t parameters;
 

ledger/src/ui/src/pin.c

@@ -24,6 +24,8 @@
 
 #include <string.h>
 
+#include "apdu.h"
+#include "os.h"
 #include "err.h"
 #include "pin.h"
 
@@ -37,25 +39,97 @@
  * Validates that the pin has exactly PIN_LENGTH alphanumeric characters
  * with at least one alphabetic character.
  *
- * @arg[in] pin null-terminated string representing the pin to validate
+ * @arg[in] pin_ctx pin context (with prepended length)
  * @ret     true if pin is valid, false otherwise
  */
-bool is_pin_valid(unsigned char *pin) {
+bool is_pin_valid(pin_t* pin_ctx) {
     // PIN_LENGTH is the only length accepted
-    size_t length = strnlen((const char *)pin, PIN_LENGTH + 1);
-    if (length != PIN_LENGTH) {
+    if (GET_PIN_LENGTH(pin_ctx) != PIN_LENGTH) {
         return false;
     }
     // Check if PIN is alphanumeric
     bool hasAlpha = false;
     for (int i = 0; i < PIN_LENGTH; i++) {
-        if (!IS_ALPHANUM(pin[i])) {
+        if (!IS_ALPHANUM(GET_PIN(pin_ctx)[i])) {
             return false;
         }
-        if (hasAlpha || IS_ALPHA(pin[i])) {
+        if (hasAlpha || IS_ALPHA(GET_PIN(pin_ctx)[i])) {
             hasAlpha = true;
         }
     }
 
     return hasAlpha;
+}
+
+/*
+ * Reset the given pin context to point to a target buffer
+ *
+ * @arg[out] pin_ctx    pin context
+ * @arg[in]  pin_buffer pin buffer to which the pin context should point
+ */
+void init_pin_ctx(pin_t* pin_ctx, unsigned char* pin_buffer) {
+    pin_ctx->pin_buffer = pin_buffer;
+}
+
+/*
+ * Reset the given pin context
+ *
+ * @arg[in] pin_ctx pin context
+ */
+void reset_pin_ctx(pin_t* pin_ctx) {
+    explicit_bzero(pin_ctx, sizeof(pin_t));
+}
+
+/*
+ * Implements RSK PIN command.
+ *
+ * Receives one byte at a time and updates the pin context, adding a null byte
+ * at the end.
+ *
+ * @arg[in] rx      number of received bytes from the Host
+ * @arg[in] pin_ctx pin context
+ * @ret             number of transmited bytes to the host
+ */
+unsigned int update_pin_buffer(volatile unsigned int rx, pin_t* pin_ctx) {
+    // Should receive 1 byte per call
+    if (APDU_DATA_SIZE(rx) != 1) {
+        THROW(PROT_INVALID);
+    }
+
+    unsigned char index = APDU_OP();
+    if ((index >= 0) && (index <= PIN_LENGTH)) {
+        pin_ctx->pin_buffer[index] = APDU_AT(DATA);
+        pin_ctx->pin_buffer[index + 1] = 0;
+    }
+
+    return 3;
+}
+
+/*
+ * Implements RSK NEW PIN command.
+ *
+ * Sets the device pin.
+ *
+ * @arg[in] rx      number of received bytes from the Host
+ * @arg[in] pin_ctx pin context
+ * @ret             number of transmited bytes to the host
+ */
+unsigned int set_device_pin(volatile unsigned int rx, pin_t* pin_ctx) {
+    // NEW_PIN command does not use any input from apdu buffer
+    UNUSED(rx);
+
+#ifndef DEBUG_BUILD
+    if (!is_pin_valid(pin_ctx)) {
+        THROW(ERR_INVALID_PIN);
+    }
+#endif
+    // Set PIN
+    os_perso_set_pin(0, GET_PIN(pin_ctx), GET_PIN_LENGTH(pin_ctx));
+    // check PIN
+    os_global_pin_invalidate();
+    unsigned char output_index = CMDPOS;
+    SET_APDU_AT(output_index++, 2);
+    SET_APDU_AT(output_index++,
+                os_global_pin_check(GET_PIN(pin_ctx), GET_PIN_LENGTH(pin_ctx)));
+    return output_index;
 }

ledger/src/ui/src/pin.h

@@ -28,7 +28,70 @@
 #include <stdbool.h>
 
 #define PIN_LENGTH 8
+#define PIN_BUFFER_LENGTH (PIN_LENGTH + 2)
 
-bool is_pin_valid(unsigned char *pin);
+// Pin context
+typedef struct {
+    unsigned char* pin_buffer;
+} pin_t;
+
+// Helper macros for pin context manipulation
+#define GET_PIN(pin_ctx) ((unsigned char*)((pin_ctx)->pin_buffer + 1))
+#define GET_PIN_LENGTH(pin_ctx) strlen((const char*)GET_PIN(pin_ctx))
+
+/*
+ * Reset the given pin context
+ *
+ * @arg[in] pin_ctx pin context
+ */
+void reset_pin_ctx(pin_t* pin_ctx);
+
+/*
+ * Reset the given pin context to point to a target buffer
+ *
+ * @arg[out] pin_ctx    pin context
+ * @arg[in]  pin_buffer pin buffer to which the pin context should point
+ */
+void init_pin_ctx(pin_t* pin_ctx, unsigned char* pin_buffer);
+
+// -----------------------------------------------------------------------
+// RSK protocol implementation
+// -----------------------------------------------------------------------
+
+/*
+ * Implements RSK PIN command.
+ *
+ * Receives one byte at a time and updates the pin context, adding a null byte
+ * at the end.
+ *
+ * @arg[in] rx      number of received bytes from the Host
+ * @arg[in] pin_ctx pin context
+ * @ret             number of transmited bytes to the host
+ */
+unsigned int update_pin_buffer(volatile unsigned int rx, pin_t* pin_ctx);
+
+/*
+ * Implements RSK NEW PIN command.
+ *
+ * Sets the device pin.
+ *
+ * @arg[in] rx      number of received bytes from the Host
+ * @arg[in] pin_ctx pin context
+ * @ret             number of transmited bytes to the host
+ */
+unsigned int set_device_pin(volatile unsigned int rx, pin_t* pin_ctx);
+
+// -----------------------------------------------------------------------
+// Pin manipulation utilities
+// -----------------------------------------------------------------------
+
+/*
+ * Validates that the pin has exactly PIN_LENGTH alphanumeric characters
+ * with at least one alphabetic character.
+ *
+ * @arg[in] pin_ctx pin context (with prepended length)
+ * @ret     true if pin is valid, false otherwise
+ */
+bool is_pin_valid(pin_t* pin_ctx);
 
 #endif

ledger/src/ui/src/unlock.c

@@ -0,0 +1,48 @@
+/**
+ * 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 "os.h"
+#include "string.h"
+#include "unlock.h"
+
+/*
+ * Implements RSK UNLOCK command.
+ *
+ * Unlocks the device.
+ *
+ * @arg[in] rx      number of received bytes from the Host
+ * @arg[in] pin_ctx pin context
+ * @ret             number of transmited bytes to the host
+ */
+unsigned int unlock(volatile unsigned int rx, pin_t *pin_ctx) {
+    // Unlock command does not use any input from apdu buffer
+    UNUSED(rx);
+
+    unsigned char output_index = OP;
+    SET_APDU_AT(output_index++,
+                os_global_pin_check(pin_ctx->pin_buffer,
+                                    strlen((const char *)pin_ctx->pin_buffer)));
+    return output_index;
+}