Merge pull request #88 from jhoenicke/pinfail

Remove PIN failure area

docs/memory.md

@@ -7,7 +7,7 @@
 | Sector  0 | 0x08000000 - 0x08003FFF |  16 KiB | boardloader (1st stage) (write-protected)
 | Sector  1 | 0x08004000 - 0x08007FFF |  16 KiB | boardloader (1st stage) (write-protected)
 | Sector  2 | 0x08008000 - 0x0800BFFF |  16 KiB | boardloader (1st stage) (write-protected)
-| Sector  3 | 0x0800C000 - 0x0800FFFF |  16 KiB | PIN counter area
+| Sector  3 | 0x0800C000 - 0x0800FFFF |  16 KiB | unused
 | Sector  4 | 0x08010000 - 0x0801FFFF |  64 KiB | storage area #1
 | Sector  5 | 0x08020000 - 0x0803FFFF | 128 KiB | bootloader (2nd stage)
 | Sector  6 | 0x08040000 - 0x0805FFFF | 128 KiB | firmware

embed/boardloader/main.c

@@ -86,6 +86,7 @@ static secbool copy_sdcard(void)
 
     // erase all flash (except boardloader)
     const uint8_t sectors[] = {
+        3,
         FLASH_SECTOR_STORAGE_1,
         FLASH_SECTOR_STORAGE_2,
         FLASH_SECTOR_BOOTLOADER,
@@ -106,7 +107,6 @@ static secbool copy_sdcard(void)
         21,
         22,
         FLASH_SECTOR_FIRMWARE_EXTRA_END,
-        FLASH_SECTOR_PIN_AREA,
     };
     if (sectrue != flash_erase_sectors(sectors, sizeof(sectors), progress_callback)) {
         display_printf(" failed\n");

embed/bootloader/messages.c

@@ -371,7 +371,6 @@ int process_msg_FirmwareUpload(uint8_t iface_num, uint32_t msg_size, uint8_t *bu
                 FLASH_SECTOR_STORAGE_2,
             };
             ensure(flash_erase_sectors(sectors_storage, sizeof(sectors_storage), NULL), NULL);
-            ensure(flash_erase_sector(FLASH_SECTOR_PIN_AREA), NULL);
         }
 
         firstskip = IMAGE_HEADER_SIZE + vhdr.hdrlen;
@@ -432,6 +431,7 @@ static void progress_wipe(int pos, int len)
 int process_msg_WipeDevice(uint8_t iface_num, uint32_t msg_size, uint8_t *buf)
 {
     const uint8_t sectors[] = {
+        3,
         FLASH_SECTOR_STORAGE_1,
         FLASH_SECTOR_STORAGE_2,
         FLASH_SECTOR_FIRMWARE_START,
@@ -451,7 +451,6 @@ int process_msg_WipeDevice(uint8_t iface_num, uint32_t msg_size, uint8_t *buf)
         21,
         22,
         FLASH_SECTOR_FIRMWARE_EXTRA_END,
-        FLASH_SECTOR_PIN_AREA,
     };
     if (sectrue != flash_erase_sectors(sectors, sizeof(sectors), progress_wipe)) {
         MSG_SEND_INIT(Failure);

embed/extmod/modtrezorconfig/modtrezorconfig.c

@@ -25,20 +25,20 @@ STATIC mp_obj_t mod_trezorconfig_init(void) {
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_0(mod_trezorconfig_init_obj, mod_trezorconfig_init);
 
-/// def unlock(pin: str) -> bool:
+/// def unlock(pin: str, waitcallback: (int -> None)) -> bool:
 ///     '''
 ///     Attempts to unlock the storage with given PIN.  Returns True on
 ///     success, False on failure.
 ///     '''
-STATIC mp_obj_t mod_trezorconfig_unlock(mp_obj_t pin) {
+STATIC mp_obj_t mod_trezorconfig_unlock(mp_obj_t pin, mp_obj_t waitcallback) {
     mp_buffer_info_t buf;
     mp_get_buffer_raise(pin, &buf, MP_BUFFER_READ);
-    if (sectrue != storage_unlock(buf.buf, buf.len)) {
+    if (sectrue != storage_unlock(buf.buf, buf.len, waitcallback)) {
         return mp_const_false;
     }
     return mp_const_true;
 }
-STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_trezorconfig_unlock_obj, mod_trezorconfig_unlock);
+STATIC MP_DEFINE_CONST_FUN_OBJ_2(mod_trezorconfig_unlock_obj, mod_trezorconfig_unlock);
 
 /// def has_pin() -> bool:
 ///     '''
@@ -52,21 +52,21 @@ STATIC mp_obj_t mod_trezorconfig_has_pin(void) {
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_0(mod_trezorconfig_has_pin_obj, mod_trezorconfig_has_pin);
 
-/// def change_pin(pin: str, newpin: str) -> bool:
+/// def change_pin(pin: str, newpin: str, waitcallback: (int -> None)) -> bool:
 ///     '''
 ///     Change PIN. Returns True on success, False on failure.
 ///     '''
-STATIC mp_obj_t mod_trezorconfig_change_pin(mp_obj_t pin, mp_obj_t newpin) {
+STATIC mp_obj_t mod_trezorconfig_change_pin(mp_obj_t pin, mp_obj_t newpin, mp_obj_t waitcallback) {
     mp_buffer_info_t pinbuf;
     mp_get_buffer_raise(pin, &pinbuf, MP_BUFFER_READ);
     mp_buffer_info_t newbuf;
     mp_get_buffer_raise(newpin, &newbuf, MP_BUFFER_READ);
-    if (sectrue != storage_change_pin(pinbuf.buf, pinbuf.len, newbuf.buf, newbuf.len)) {
+    if (sectrue != storage_change_pin(pinbuf.buf, pinbuf.len, newbuf.buf, newbuf.len, waitcallback)) {
         return mp_const_false;
     }
     return mp_const_true;
 }
-STATIC MP_DEFINE_CONST_FUN_OBJ_2(mod_trezorconfig_change_pin_obj, mod_trezorconfig_change_pin);
+STATIC MP_DEFINE_CONST_FUN_OBJ_3(mod_trezorconfig_change_pin_obj, mod_trezorconfig_change_pin);
 
 /// def get(app: int, key: int) -> bytes:
 ///     '''

embed/extmod/modtrezorconfig/norcow.c

@@ -279,3 +279,21 @@ secbool norcow_set(uint16_t key, const void *val, uint16_t len)
     }
     return r;
 }
+
+/*
+ * Update a word in flash at the given pointer.  The pointer must point
+ * into the NORCOW area.
+ */
+secbool norcow_update(uint16_t key, uint16_t offset, uint32_t value)
+{
+    const void *ptr;
+    uint16_t len;
+    if (sectrue != find_item(norcow_active_sector, key, &ptr, &len)) {
+        return secfalse;
+    }
+    if ((offset & 3) != 0 || offset >= len) {
+        return secfalse;
+    }
+    uint32_t sector_offset = (const uint8_t*) ptr - (const uint8_t *)norcow_ptr(norcow_active_sector, 0, NORCOW_SECTOR_SIZE) + offset;
+    return flash_write_word_rel(norcow_sectors[norcow_active_sector], sector_offset, value);
+}

embed/extmod/modtrezorconfig/norcow.h

@@ -31,4 +31,10 @@ secbool norcow_get(uint16_t key, const void **val, uint16_t *len);
  */
 secbool norcow_set(uint16_t key, const void *val, uint16_t len);
 
+/*
+ * Update a word in flash in the given key at the given offset.
+ * Note that you can only change bits from 1 to 0.
+ */
+secbool norcow_update(uint16_t key, uint16_t offset, uint32_t value);
+
 #endif

embed/extmod/modtrezorconfig/storage.c

@@ -10,12 +10,8 @@
 #include "common.h"
 #include "norcow.h"
 #include "../../trezorhal/flash.h"
-
-// Byte-length of flash sector containing fail counters.
-#define PIN_SECTOR_SIZE 0x4000
-
-// Maximum number of failed unlock attempts.
-#define PIN_MAX_TRIES 15
+#include "py/runtime.h"
+#include "py/obj.h"
 
 // Norcow storage key of configured PIN.
 #define PIN_KEY 0x0000
@@ -23,6 +19,13 @@
 // Maximum PIN length.
 #define PIN_MAXLEN 32
 
+// Byte-length of flash section containing fail counters.
+#define PIN_FAIL_KEY 0x0001
+#define PIN_FAIL_SECTOR_SIZE 32
+
+// Maximum number of failed unlock attempts.
+#define PIN_MAX_TRIES 15
+
 static secbool initialized = secfalse;
 static secbool unlocked = secfalse;
 
@@ -35,41 +38,24 @@ void storage_init(void)
     initialized = sectrue;
 }
 
-static void pin_fails_reset(uint32_t ofs)
+static void pin_fails_reset(uint16_t ofs)
 {
-    if (ofs + sizeof(uint32_t) >= PIN_SECTOR_SIZE) {
-        // ofs points to the last word of the PIN fails area.  Because there is
-        // no space left, we recycle the sector (set all words to 0xffffffff).
-        // On next unlock attempt, we start counting from the the first word.
-        flash_erase_sector(FLASH_SECTOR_PIN_AREA);
-    } else {
-        // Mark this counter as exhausted.  On next unlock attempt, pinfails_get
-        // seeks to the next word.
-        flash_unlock();
-        flash_write_word_rel(FLASH_SECTOR_PIN_AREA, ofs, 0);
-        flash_lock();
-    }
+    norcow_update(PIN_FAIL_KEY, ofs, 0);
 }
 
-static secbool pin_fails_increase(uint32_t ofs)
+static secbool pin_fails_increase(const uint32_t *ptr, uint16_t ofs)
 {
-    uint32_t ctr = ~PIN_MAX_TRIES;
-    if (sectrue != flash_read_word_rel(FLASH_SECTOR_PIN_AREA, ofs, &ctr)) {
-        return secfalse;
-    }
+    uint32_t ctr = *ptr;
     ctr = ctr << 1;
 
     flash_unlock();
-    if (sectrue != flash_write_word_rel(FLASH_SECTOR_PIN_AREA, ofs, ctr)) {
+    if (sectrue != norcow_update(PIN_FAIL_KEY, ofs, ctr)) {
         flash_lock();
         return secfalse;
     }
     flash_lock();
 
-    uint32_t check = 0;
-    if (sectrue != flash_read_word_rel(FLASH_SECTOR_PIN_AREA, ofs, &check)) {
-        return secfalse;
-    }
+    uint32_t check = *ptr;
     if (ctr != check) {
         return secfalse;
     }
@@ -84,25 +70,6 @@ static void pin_fails_check_max(uint32_t ctr)
     }
 }
 
-static secbool pin_fails_read(uint32_t *ofs, uint32_t *ctr)
-{
-    if (NULL == ofs || NULL == ctr) {
-        return secfalse;
-    }
-    for (uint32_t o = 0; o < PIN_SECTOR_SIZE; o += sizeof(uint32_t)) {
-        uint32_t c = 0;
-        if (!flash_read_word_rel(FLASH_SECTOR_PIN_AREA, o, &c)) {
-            return secfalse;
-        }
-        if (c != 0) {
-            *ofs = o;
-            *ctr = c;
-            return sectrue;
-        }
-    }
-    return secfalse;
-}
-
 static secbool const_cmp(const uint8_t *pub, size_t publen, const uint8_t *sec, size_t seclen)
 {
     size_t diff = seclen ^ publen;
@@ -124,39 +91,89 @@ static secbool pin_cmp(const uint8_t *pin, size_t pinlen)
     }
 }
 
-static secbool pin_check(const uint8_t *pin, size_t len)
+static secbool pin_get_fails(const uint32_t **pinfail, uint32_t *pofs)
 {
-    uint32_t ofs;
-    uint32_t ctr;
-    if (sectrue != pin_fails_read(&ofs, &ctr)) {
+    const void *vpinfail;
+    uint16_t pinfaillen;
+    unsigned int ofs;
+    // The PIN_FAIL_KEY points to an area of words, initialized to
+    // 0xffffffff (meaning no pin failures).  The first non-zero word
+    // in this area is the current pin failure counter.  If  PIN_FAIL_KEY
+    // has no configuration or is empty, the pin failure counter is 0.
+    // We rely on the fact that flash allows to clear bits and we clear one
+    // bit to indicate pin failure.  On success, the word is set to 0,
+    // indicating that the next word is the pin failure counter.
+
+    // Find the current pin failure counter
+    if (secfalse != norcow_get(PIN_FAIL_KEY, &vpinfail, &pinfaillen)) {
+        *pinfail = vpinfail;
+        for (ofs = 0; ofs < pinfaillen / sizeof(uint32_t); ofs++) {
+            if (((const uint32_t *) vpinfail)[ofs]) {
+                *pinfail = vpinfail;
+                *pofs = ofs;
+                return sectrue;
+            }
+        }
+    }
+
+    // No pin failure section, or all entries used -> create a new one.
+    uint32_t pinarea[PIN_FAIL_SECTOR_SIZE];
+    memset(pinarea, 0xff, sizeof(pinarea));
+    if (sectrue != norcow_set(PIN_FAIL_KEY, pinarea, sizeof(pinarea))) {
         return secfalse;
     }
+    if (sectrue != norcow_get(PIN_FAIL_KEY, &vpinfail, &pinfaillen)) {
+        return secfalse;
+    }
+    *pinfail = vpinfail;
+    *pofs = 0;
+    return sectrue;
+}
+
+static secbool pin_check(const uint8_t *pin, size_t len, mp_obj_t callback)
+{
+    const uint32_t *pinfail = NULL;
+    uint32_t ofs;
+    uint32_t ctr;
+
+    // Get the pin failure counter
+    if (pin_get_fails(&pinfail, &ofs) != sectrue) {
+        return secfalse;
+    }
+
+    // Read current failure counter
+    ctr = pinfail[ofs];
+    // Wipe storage if too many failures
     pin_fails_check_max(ctr);
 
     // Sleep for ~ctr seconds before checking the PIN.
     for (uint32_t wait = ~ctr; wait > 0; wait--) {
+        mp_obj_t waitobj = mp_obj_new_int(wait);
+        mp_call_function_1(callback, waitobj);
         hal_delay(1000);
     }
 
     // First, we increase PIN fail counter in storage, even before checking the
     // PIN.  If the PIN is correct, we reset the counter afterwards.  If not, we
     // check if this is the last allowed attempt.
-    if (sectrue != pin_fails_increase(ofs)) {
+    if (sectrue != pin_fails_increase(pinfail + ofs, ofs * sizeof(uint32_t))) {
         return secfalse;
     }
     if (sectrue != pin_cmp(pin, len)) {
+        // Wipe storage if too many failures
         pin_fails_check_max(ctr << 1);
         return secfalse;
     }
-    pin_fails_reset(ofs);
+    // Finally set the counter to 0 to indicate success.
+    pin_fails_reset(ofs * sizeof(uint32_t));
 
     return sectrue;
 }
 
-secbool storage_unlock(const uint8_t *pin, size_t len)
+secbool storage_unlock(const uint8_t *pin, size_t len, mp_obj_t callback)
 {
     unlocked = secfalse;
-    if (sectrue == initialized && sectrue == pin_check(pin, len)) {
+    if (sectrue == initialized && sectrue == pin_check(pin, len, callback)) {
         unlocked = sectrue;
     }
     return unlocked;
@@ -164,7 +181,7 @@ secbool storage_unlock(const uint8_t *pin, size_t len)
 
 secbool storage_get(uint16_t key, const void **val, uint16_t *len)
 {
-    if (sectrue != initialized || sectrue != unlocked || PIN_KEY == key) {
+    if (sectrue != initialized || sectrue != unlocked || (key >> 8) == 0) {
         return secfalse;
     }
     return norcow_get(key, val, len);
@@ -172,7 +189,7 @@ secbool storage_get(uint16_t key, const void **val, uint16_t *len)
 
 secbool storage_set(uint16_t key, const void *val, uint16_t len)
 {
-    if (sectrue != initialized || sectrue != unlocked || PIN_KEY == key) {
+    if (sectrue != initialized || sectrue != unlocked || (key >> 8) == 0) {
         return secfalse;
     }
     return norcow_set(key, val, len);
@@ -189,12 +206,12 @@ secbool storage_has_pin(void)
     return sectrue * (0 != spinlen);
 }
 
-secbool storage_change_pin(const uint8_t *pin, size_t len, const uint8_t *newpin, size_t newlen)
+secbool storage_change_pin(const uint8_t *pin, size_t len, const uint8_t *newpin, size_t newlen, mp_obj_t callback)
 {
     if (sectrue != initialized || sectrue != unlocked || newlen > PIN_MAXLEN) {
         return secfalse;
     }
-    if (sectrue != pin_check(pin, len)) {
+    if (sectrue != pin_check(pin, len, callback)) {
         return secfalse;
     }
     return norcow_set(PIN_KEY, newpin, newlen);

embed/extmod/modtrezorconfig/storage.h

@@ -8,11 +8,13 @@
 #include <stdint.h>
 #include <stddef.h>
 #include "../../trezorhal/secbool.h"
+#include "py/obj.h"
 
 void storage_init(void);
 void storage_wipe(void);
-secbool storage_unlock(const uint8_t *pin, size_t len);
+secbool storage_unlock(const uint8_t *pin, size_t len, mp_obj_t callback);
 secbool storage_has_pin(void);
-secbool storage_change_pin(const uint8_t *pin, size_t len, const uint8_t *newpin, size_t newlen);
+uint32_t storage_pin_wait_time(void);
+secbool storage_change_pin(const uint8_t *pin, size_t len, const uint8_t *newpin, size_t newlen, mp_obj_t callback);
 secbool storage_get(uint16_t key, const void **val, uint16_t *len);
 secbool storage_set(uint16_t key, const void *val, uint16_t len);

embed/trezorhal/flash.h

@@ -10,7 +10,7 @@
 //                                           1
 #define FLASH_SECTOR_BOARDLOADER_END         2
 
-#define FLASH_SECTOR_PIN_AREA                3
+//                                           3
 
 #define FLASH_SECTOR_STORAGE_1               4
 

src/apps/management/change_pin.py

@@ -62,7 +62,11 @@ async def layout_change_pin(ctx, msg):
     else:
         new_pin = await request_pin_confirm(ctx)
 
-    config.change_pin(curr_pin, new_pin)
+    def show_timeout(wait):
+        # TODO
+        return
+
+    config.change_pin(curr_pin, new_pin, show_timeout)
 
     if new_pin:
         return Success(message='PIN changed')

src/boot.py

@@ -5,13 +5,19 @@ from trezor import ui
 from apps.common.request_pin import request_pin
 
 
+def show_timeout(wait):
+    # TODO
+    from trezor import log
+    log.debug('PIN', 'waiting %d seconds', wait)
+
+
 async def unlock_layout():
     while True:
         if config.has_pin():
             pin = await request_pin()
         else:
             pin = ''
-        if config.unlock(pin):
+        if config.unlock(pin, show_timeout):
             return
         else:
             await unlock_failed()