From 497021f2ef30b0f99e5190fdec51e70e1131a15c Mon Sep 17 00:00:00 2001 From: Jochen Hoenicke Date: Tue, 12 Dec 2017 23:47:42 +0100 Subject: [PATCH 1/3] storage: New pin fail section inside NORCOW Added a function to update NORCOW data. Changed storage pin fail logic. --- embed/extmod/modtrezorconfig/norcow.c | 18 ++++ embed/extmod/modtrezorconfig/norcow.h | 6 ++ embed/extmod/modtrezorconfig/storage.c | 112 +++++++++++++------------ 3 files changed, 81 insertions(+), 55 deletions(-) diff --git a/embed/extmod/modtrezorconfig/norcow.c b/embed/extmod/modtrezorconfig/norcow.c index ab0758c500..dbc0faef6c 100644 --- a/embed/extmod/modtrezorconfig/norcow.c +++ b/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); +} diff --git a/embed/extmod/modtrezorconfig/norcow.h b/embed/extmod/modtrezorconfig/norcow.h index 08bebf0f81..6711b4b041 100644 --- a/embed/extmod/modtrezorconfig/norcow.h +++ b/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 diff --git a/embed/extmod/modtrezorconfig/storage.c b/embed/extmod/modtrezorconfig/storage.c index d9f180c5a1..e1dda88541 100644 --- a/embed/extmod/modtrezorconfig/storage.c +++ b/embed/extmod/modtrezorconfig/storage.c @@ -11,18 +11,19 @@ #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 - // Norcow storage key of configured PIN. #define PIN_KEY 0x0000 // 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 +36,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 +68,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; @@ -126,11 +91,47 @@ static secbool pin_cmp(const uint8_t *pin, size_t pinlen) static secbool pin_check(const uint8_t *pin, size_t len) { - uint32_t ofs; + uint32_t ofs = 0; uint32_t ctr; - if (sectrue != pin_fails_read(&ofs, &ctr)) { - return secfalse; + const void *vpinfail; + const uint32_t *pinfail = NULL; + uint16_t pinfaillen; + + // 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 + secbool found = secfalse; + if (secfalse != norcow_get(PIN_FAIL_KEY, &vpinfail, &pinfaillen)) { + pinfail = vpinfail; + for (ofs = 0; ofs < pinfaillen / sizeof(uint32_t); ofs++) { + if (pinfail[ofs]) { + found = sectrue; + break; + } + } } + if (found == secfalse) { + // 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; + ofs = 0; + } + + // Read current failure counter + ctr = pinfail[ofs]; pin_fails_check_max(ctr); // Sleep for ~ctr seconds before checking the PIN. @@ -141,14 +142,15 @@ static secbool pin_check(const uint8_t *pin, size_t len) // 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)) { 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; } @@ -164,7 +166,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 +174,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); From 9f2bbb0e1a1bad15cb5a1be5ea7487bbf52fa1e2 Mon Sep 17 00:00:00 2001 From: Jochen Hoenicke Date: Wed, 13 Dec 2017 00:11:25 +0100 Subject: [PATCH 2/3] Removed references to pin fail area --- docs/memory.md | 2 +- embed/boardloader/main.c | 2 +- embed/bootloader/messages.c | 3 +-- embed/trezorhal/flash.h | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/memory.md b/docs/memory.md index 35d7db9075..9cb27e9df2 100644 --- a/docs/memory.md +++ b/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 diff --git a/embed/boardloader/main.c b/embed/boardloader/main.c index bdce3a2911..20e2109bad 100644 --- a/embed/boardloader/main.c +++ b/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"); diff --git a/embed/bootloader/messages.c b/embed/bootloader/messages.c index e62f67da4d..5006cb0f8e 100644 --- a/embed/bootloader/messages.c +++ b/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); diff --git a/embed/trezorhal/flash.h b/embed/trezorhal/flash.h index fcaa102a4e..9fc9c4ddc3 100644 --- a/embed/trezorhal/flash.h +++ b/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 From 87f7054e4630a39bbf79bde627c90a9823ca7da6 Mon Sep 17 00:00:00 2001 From: Jochen Hoenicke Date: Thu, 14 Dec 2017 17:14:15 +0100 Subject: [PATCH 3/3] Added callback for PIN timeout When PIN is entered or changed and their were failed tries the function waits for time (exponential slow down). For every second it waits, it now calls back into python to give it the chance to show a message. GUI still needs to be implemented --- .../extmod/modtrezorconfig/modtrezorconfig.c | 16 ++--- embed/extmod/modtrezorconfig/storage.c | 67 ++++++++++++------- embed/extmod/modtrezorconfig/storage.h | 6 +- src/apps/management/change_pin.py | 6 +- src/boot.py | 8 ++- 5 files changed, 65 insertions(+), 38 deletions(-) diff --git a/embed/extmod/modtrezorconfig/modtrezorconfig.c b/embed/extmod/modtrezorconfig/modtrezorconfig.c index 5fb0c03ca2..2f082c7db6 100644 --- a/embed/extmod/modtrezorconfig/modtrezorconfig.c +++ b/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: /// ''' diff --git a/embed/extmod/modtrezorconfig/storage.c b/embed/extmod/modtrezorconfig/storage.c index e1dda88541..25a2456d3d 100644 --- a/embed/extmod/modtrezorconfig/storage.c +++ b/embed/extmod/modtrezorconfig/storage.c @@ -10,6 +10,8 @@ #include "common.h" #include "norcow.h" #include "../../trezorhal/flash.h" +#include "py/runtime.h" +#include "py/obj.h" // Norcow storage key of configured PIN. #define PIN_KEY 0x0000 @@ -89,14 +91,11 @@ 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 = 0; - uint32_t ctr; const void *vpinfail; - const uint32_t *pinfail = NULL; 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 @@ -106,36 +105,51 @@ static secbool pin_check(const uint8_t *pin, size_t len) // indicating that the next word is the pin failure counter. // Find the current pin failure counter - secbool found = secfalse; if (secfalse != norcow_get(PIN_FAIL_KEY, &vpinfail, &pinfaillen)) { - pinfail = vpinfail; + *pinfail = vpinfail; for (ofs = 0; ofs < pinfaillen / sizeof(uint32_t); ofs++) { - if (pinfail[ofs]) { - found = sectrue; - break; + if (((const uint32_t *) vpinfail)[ofs]) { + *pinfail = vpinfail; + *pofs = ofs; + return sectrue; } } } - if (found == secfalse) { - // 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; - ofs = 0; + + // 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); } @@ -146,6 +160,7 @@ static secbool pin_check(const uint8_t *pin, size_t len) return secfalse; } if (sectrue != pin_cmp(pin, len)) { + // Wipe storage if too many failures pin_fails_check_max(ctr << 1); return secfalse; } @@ -155,10 +170,10 @@ static secbool pin_check(const uint8_t *pin, size_t len) 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; @@ -191,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); diff --git a/embed/extmod/modtrezorconfig/storage.h b/embed/extmod/modtrezorconfig/storage.h index 7fc9eef4c1..709d4d6f66 100644 --- a/embed/extmod/modtrezorconfig/storage.h +++ b/embed/extmod/modtrezorconfig/storage.h @@ -8,11 +8,13 @@ #include #include #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); diff --git a/src/apps/management/change_pin.py b/src/apps/management/change_pin.py index f1d4cc27c2..e5d90f5785 100644 --- a/src/apps/management/change_pin.py +++ b/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') diff --git a/src/boot.py b/src/boot.py index d3afefc176..0086e80869 100644 --- a/src/boot.py +++ b/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()