More adjustments after code review

- Reworked retries test to just assert that the returned value is valid
- Changed some function names for clarity
- Removed retries counter logic from mock
- Added mock variables to assert that wipe and unlock operations are done while the device is locked
- Added a test case to assert that pin capping is applied

ledger/src/ui/test/communication/test_communication.c

@@ -46,45 +46,8 @@ void test_get_mode() {
 void test_get_retries() {
     printf("Test get retries...\n");
 
-    unsigned char pin_buffer[] = "X1234567a";
-    unsigned char wrong_pin[] = "Xa7654321";
-    unsigned int rx = 4;
-    init_mock_ctx();
-    for (int i = 0; i < strlen((const char *)pin_buffer); i++) {
-        SET_APDU_AT(2, i);
-        SET_APDU_AT(3, pin_buffer[i]);
-        assert(3 == update_pin_buffer(rx));
-    }
-
-    assert(3 == set_pin());
-    assert(3 == get_retries());
-    assert(0 == APDU_AT(2));
-
-    // Send wrong pin
-    for (int i = 0; i < strlen((const char *)wrong_pin); i++) {
-        SET_APDU_AT(2, i);
-        SET_APDU_AT(3, wrong_pin[i]);
-        assert(3 == update_pin_buffer(rx));
-    }
-    assert(!unlock_with_pin(true));
-    assert(3 == get_retries());
-    assert(1 == APDU_AT(2));
-    assert(!unlock_with_pin(true));
-    assert(3 == get_retries());
-    assert(2 == APDU_AT(2));
-    assert(!unlock_with_pin(true));
-    assert(3 == get_retries());
-    assert(3 == APDU_AT(2));
-
-    // Send right pin again
-    for (int i = 0; i < strlen((const char *)pin_buffer); i++) {
-        SET_APDU_AT(2, i);
-        SET_APDU_AT(3, pin_buffer[i]);
-        assert(3 == update_pin_buffer(rx));
-    }
-    assert(unlock_with_pin(true));
     assert(3 == get_retries());
-    assert(0 == APDU_AT(2));
+    assert(MOCK_INTERNAL_RETRIES_COUNTER == APDU_AT(2));
 }
 
 int main() {

ledger/src/ui/test/mock/cx.c

@@ -26,7 +26,7 @@
 
 static unsigned char mock_seed[32];
 
-void set_mock_seed(const unsigned char *data, unsigned int len) {
+void mock_cx_rng(const unsigned char *data, unsigned int len) {
     for (unsigned int i = 0; i < len; i++) {
         mock_seed[i] = data[i];
     }

ledger/src/ui/test/mock/cx.h

@@ -27,4 +27,4 @@
  */
 unsigned char *cx_rng(unsigned char *buffer, unsigned int len);
 
-void set_mock_seed(const unsigned char *data, unsigned int len);
+void mock_cx_rng(const unsigned char *data, unsigned int len);

ledger/src/ui/test/mock/os.c

@@ -54,16 +54,18 @@ void explicit_bzero(void *s, size_t len) {
 
 unsigned int os_global_pin_check(unsigned char *pin_buffer,
                                  unsigned char pin_length) {
-    mock_ctx.device_unlocked = !strncmp((const char *)pin_buffer,
-                                        (const char *)mock_ctx.global_pin,
-                                        pin_length);
-    if (mock_ctx.device_unlocked) {
-        mock_ctx.retries = 0;
-    } else {
-        mock_ctx.retries++;
+    bool pin_matches = !strncmp((const char *)pin_buffer,
+                                (const char *)mock_ctx.global_pin,
+                                pin_length);
+
+    // Assert that unlock was performed while the device was locked
+    if (pin_matches && !mock_ctx.device_unlocked) {
+        mock_ctx.successful_unlock_while_locked_count++;
     }
+    // Update mock state
+    mock_ctx.device_unlocked = pin_matches;
 
-    return mock_ctx.device_unlocked;
+    return (int)pin_matches;
 }
 
 void os_perso_set_pin(unsigned int identity,
@@ -91,8 +93,14 @@ void nvm_write(void *dst_adr, void *src_adr, unsigned int src_len) {
 }
 
 void os_perso_wipe() {
+    if (!mock_ctx.device_unlocked) {
+        mock_ctx.wipe_while_locked_count++;
+    }
     // wipe global pin, seed and state
-    init_mock_ctx();
+    memset(mock_ctx.global_pin, 0x0, sizeof(mock_ctx.global_pin));
+    memset(mock_ctx.global_seed, 0x0, sizeof(mock_ctx.global_seed));
+    mock_ctx.device_unlocked = false;
+    mock_ctx.device_onboarded = false;
 }
 
 void os_perso_finalize(void) {
@@ -104,7 +112,7 @@ unsigned int os_perso_isonboarded(void) {
 }
 
 unsigned int os_global_pin_retries(void) {
-    return mock_ctx.retries;
+    return (unsigned int)MOCK_INTERNAL_RETRIES_COUNTER;
 }
 
 // Generated mnemonics buffer will be "mnemonics-generated-from:<in>"

ledger/src/ui/test/mock/os.h

@@ -40,6 +40,10 @@
 #define IO_APDU_BUFFER_SIZE 85
 extern unsigned char G_io_apdu_buffer[IO_APDU_BUFFER_SIZE];
 
+/**
+ * Mock internal retries counter
+ */
+#define MOCK_INTERNAL_RETRIES_COUNTER 123
 /**
  * Mock context used to assert current state
  */
@@ -48,7 +52,8 @@ typedef struct {
     unsigned char global_seed[257];
     bool device_unlocked;
     bool device_onboarded;
-    unsigned int retries;
+    unsigned int wipe_while_locked_count;
+    unsigned int successful_unlock_while_locked_count;
 } mock_ctx_t;
 
 void init_mock_ctx();

ledger/src/ui/test/onboard/test_onboard.c

@@ -111,16 +111,16 @@ void test_onboard_device() {
     }
 
     init_mock_ctx();
-    set_mock_seed(seed, SEEDSIZE);
+    mock_cx_rng(seed, SEEDSIZE);
     assert(3 == onboard_device(&onboard_ctx));
     assert(2 == APDU_AT(1));
     assert(1 == APDU_AT(2));
 
     mock_ctx_t mock_ctx;
     get_mock_ctx(&mock_ctx);
-    assert(mock_ctx.device_unlocked == true);
-    assert(mock_ctx.device_onboarded == true);
-    assert(mock_ctx.retries == 0);
+    assert(true == mock_ctx.device_unlocked);
+    assert(true == mock_ctx.device_onboarded);
+    assert(1 == mock_ctx.wipe_while_locked_count);
     assert(!strcmp((const char *)(valid_pin + 1),
                    (const char *)mock_ctx.global_pin));
     assert(!strcmp((const char *)expected_global_seed,
@@ -135,7 +135,7 @@ void test_onboard_device() {
                   onboard_ctx.words_buffer,
                   sizeof(expected_words_buffer)) == 0);
     assert(memcmp(expected_seed, onboard_ctx.seed, sizeof(expected_seed)) == 0);
-    assert(onboard_ctx.words_buffer_length == 0);
+    assert(0 == onboard_ctx.words_buffer_length);
 }
 
 void test_onboard_device_invalid_pin() {
@@ -178,7 +178,6 @@ void test_onboard_device_invalid_pin() {
 
     assert(false == mock_ctx.device_onboarded);
     assert(false == mock_ctx.device_unlocked);
-    assert(0 == mock_ctx.retries);
     assert(0 == memcmp(expected_global_pin,
                        mock_ctx.global_pin,
                        sizeof(expected_global_pin)));

ledger/src/ui/test/pin/test_pin.c

@@ -133,6 +133,7 @@ void test_set_pin() {
     mock_ctx_t mock_ctx;
     get_mock_ctx(&mock_ctx);
     assert(true == mock_ctx.device_unlocked);
+    assert(1 == mock_ctx.successful_unlock_while_locked_count);
 }
 
 void test_set_pin_invalid() {
@@ -172,6 +173,29 @@ void test_unlock_with_pin() {
     assert(!strcmp((const char *)(pin_buffer + 1),
                    (const char *)mock_ctx.global_pin));
     assert(true == mock_ctx.device_unlocked);
+    assert(1 == mock_ctx.successful_unlock_while_locked_count);
+}
+
+void test_unlock_with_pin_capping() {
+    printf("Test unlock with pin capping...\n");
+
+    unsigned char pin_buffer[] = "X1234567abcdef";
+    unsigned int rx = 4;
+    init_mock_ctx();
+    for (int i = 0; i < strlen((const char *)pin_buffer); i++) {
+        SET_APDU_AT(2, i);
+        SET_APDU_AT(3, pin_buffer[i]);
+        assert(3 == update_pin_buffer(rx));
+    }
+    assert(3 == set_pin());
+    assert(1 == unlock_with_pin(true));
+    mock_ctx_t mock_ctx;
+    get_mock_ctx(&mock_ctx);
+    // Make sure pin capping is applied
+    // (i.e. only the first 8 bytes are copied to global buffer)
+    assert(!strcmp("1234567a", (const char *)mock_ctx.global_pin));
+    assert(true == mock_ctx.device_unlocked);
+    assert(1 == mock_ctx.successful_unlock_while_locked_count);
 }
 
 void test_unlock_with_pin_not_set() {
@@ -190,6 +214,7 @@ void test_unlock_with_pin_not_set() {
     get_mock_ctx(&mock_ctx);
     assert(0 == unlock_with_pin(true));
     assert(false == mock_ctx.device_unlocked);
+    assert(0 == mock_ctx.successful_unlock_while_locked_count);
     const char *expected_global_pin[sizeof(mock_ctx.global_pin)];
     memset(expected_global_pin, 0, sizeof(expected_global_pin));
     assert(!strcmp((const char *)expected_global_pin,
@@ -227,6 +252,7 @@ int main() {
     test_set_pin_invalid();
     test_validate_ok();
     test_unlock_with_pin();
+    test_unlock_with_pin_capping();
     test_unlock_with_pin_not_set();
     test_set_device_pin();