From c0cd252c8384004fd99b7447ce54989919b35ce6 Mon Sep 17 00:00:00 2001 From: Jochen Hoenicke Date: Fri, 15 Dec 2017 10:27:36 +0100 Subject: [PATCH] Store pin as integer - New module trezor.pin (add timeout here) - Convert pin to integer by adding a '1' (to detect leading 0s) - pin is still limited to 9 digits. --- .../extmod/modtrezorconfig/modtrezorconfig.c | 17 ++++------ embed/extmod/modtrezorconfig/storage.c | 33 +++++++------------ embed/extmod/modtrezorconfig/storage.h | 4 +-- src/apps/management/change_pin.py | 14 +++++--- src/apps/management/load_device.py | 3 +- src/boot.py | 3 +- src/trezor/pin.py | 6 ++++ 7 files changed, 40 insertions(+), 40 deletions(-) create mode 100644 src/trezor/pin.py diff --git a/embed/extmod/modtrezorconfig/modtrezorconfig.c b/embed/extmod/modtrezorconfig/modtrezorconfig.c index de0275cf84..6c1f5490c2 100644 --- a/embed/extmod/modtrezorconfig/modtrezorconfig.c +++ b/embed/extmod/modtrezorconfig/modtrezorconfig.c @@ -25,15 +25,14 @@ 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, waitcallback: (int, int -> None)) -> bool: +/// def unlock(pin: int, waitcallback: (int, 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, 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, waitcallback)) { + uint32_t pin_i = mp_obj_get_int(pin); + if (sectrue != storage_unlock(pin_i, waitcallback)) { return mp_const_false; } return mp_const_true; @@ -52,16 +51,14 @@ 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, waitcallback: (int, int -> None)) -> bool: +/// def change_pin(pin: int, newpin: int, waitcallback: (int, 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, 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, waitcallback)) { + uint32_t pin_i = mp_obj_get_int(pin); + uint32_t newpin_i = mp_obj_get_int(newpin); + if (sectrue != storage_change_pin(pin_i, newpin_i, waitcallback)) { return mp_const_false; } return mp_const_true; diff --git a/embed/extmod/modtrezorconfig/storage.c b/embed/extmod/modtrezorconfig/storage.c index 3959be581f..543be4a6ed 100644 --- a/embed/extmod/modtrezorconfig/storage.c +++ b/embed/extmod/modtrezorconfig/storage.c @@ -71,24 +71,15 @@ static void pin_fails_check_max(uint32_t ctr) } } -static secbool const_cmp(const uint8_t *pub, size_t publen, const uint8_t *sec, size_t seclen) -{ - size_t diff = seclen ^ publen; - for (size_t i = 0; i < publen; i++) { - diff |= pub[i] ^ sec[i]; - } - return sectrue * (0 == diff); -} - -static secbool pin_cmp(const uint8_t *pin, size_t pinlen) +static secbool pin_cmp(const uint32_t pin) { const void *spin = NULL; uint16_t spinlen = 0; norcow_get(PIN_KEY, &spin, &spinlen); - if (NULL != spin) { - return const_cmp(pin, pinlen, spin, spinlen); + if (NULL != spin && spinlen == sizeof(uint32_t)) { + return sectrue * (pin == *(const uint32_t*)spin); } else { - return sectrue * (0 == pinlen); + return sectrue * (1 == pin); } } @@ -131,7 +122,7 @@ static secbool pin_get_fails(const uint32_t **pinfail, uint32_t *pofs) return sectrue; } -static secbool pin_check(const uint8_t *pin, size_t len, mp_obj_t callback) +static secbool pin_check(uint32_t pin, mp_obj_t callback) { const uint32_t *pinfail = NULL; uint32_t ofs; @@ -161,7 +152,7 @@ static secbool pin_check(const uint8_t *pin, size_t len, mp_obj_t callback) if (sectrue != pin_fails_increase(pinfail + ofs, ofs * sizeof(uint32_t))) { return secfalse; } - if (sectrue != pin_cmp(pin, len)) { + if (sectrue != pin_cmp(pin)) { // Wipe storage if too many failures pin_fails_check_max(ctr << 1); return secfalse; @@ -172,10 +163,10 @@ static secbool pin_check(const uint8_t *pin, size_t len, mp_obj_t callback) return sectrue; } -secbool storage_unlock(const uint8_t *pin, size_t len, mp_obj_t callback) +secbool storage_unlock(const uint32_t pin, mp_obj_t callback) { unlocked = secfalse; - if (sectrue == initialized && sectrue == pin_check(pin, len, callback)) { + if (sectrue == initialized && sectrue == pin_check(pin, callback)) { unlocked = sectrue; } return unlocked; @@ -208,15 +199,15 @@ 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, mp_obj_t callback) +secbool storage_change_pin(const uint32_t pin, const uint32_t newpin, mp_obj_t callback) { - if (sectrue != initialized || sectrue != unlocked || newlen > PIN_MAXLEN) { + if (sectrue != initialized || sectrue != unlocked) { return secfalse; } - if (sectrue != pin_check(pin, len, callback)) { + if (sectrue != pin_check(pin, callback)) { return secfalse; } - return norcow_set(PIN_KEY, newpin, newlen); + return norcow_set(PIN_KEY, &newpin, sizeof(uint32_t)); } void storage_wipe(void) diff --git a/embed/extmod/modtrezorconfig/storage.h b/embed/extmod/modtrezorconfig/storage.h index 670d4f0af7..75bfeed6ce 100644 --- a/embed/extmod/modtrezorconfig/storage.h +++ b/embed/extmod/modtrezorconfig/storage.h @@ -12,9 +12,9 @@ void storage_init(void); void storage_wipe(void); -secbool storage_unlock(const uint8_t *pin, size_t len, mp_obj_t callback); +secbool storage_unlock(const uint32_t pin, mp_obj_t callback); secbool storage_has_pin(void); 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_change_pin(const uint32_t pin, const uint32_t newpin, 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 3f39e565c3..5429021c53 100644 --- a/src/apps/management/change_pin.py +++ b/src/apps/management/change_pin.py @@ -1,5 +1,6 @@ from trezor import ui from trezor import config +from trezor.pin import pin_to_int from trezor.utils import unimport @@ -50,6 +51,8 @@ def confirm_change_pin(ctx, msg): @unimport async def layout_change_pin(ctx, msg): from trezor.messages.Success import Success + from trezor.messages.Failure import Failure + from trezor.messages.FailureType import PinInvalid from apps.common.request_pin import show_pin_timeout await confirm_change_pin(ctx, msg) @@ -63,9 +66,10 @@ async def layout_change_pin(ctx, msg): else: new_pin = await request_pin_confirm(ctx) - config.change_pin(curr_pin, new_pin, show_pin_timeout) - - if new_pin: - return Success(message='PIN changed') + if config.change_pin(pin_to_int(curr_pin), pin_to_int(new_pin), show_pin_timeout): + if new_pin: + return Success(message='PIN changed') + else: + return Success(message='PIN removed') else: - return Success(message='PIN removed') + return Failure(code=PinInvalid, message='PIN invalid') diff --git a/src/apps/management/load_device.py b/src/apps/management/load_device.py index 3a918390af..78567550cd 100644 --- a/src/apps/management/load_device.py +++ b/src/apps/management/load_device.py @@ -1,4 +1,5 @@ from trezor import wire, ui, config +from trezor.pin import pin_to_int from trezor.utils import unimport @@ -29,6 +30,6 @@ async def layout_load_device(ctx, msg): storage.load_settings(use_passphrase=msg.passphrase_protection, label=msg.label) if msg.pin: - config.change_pin('', msg.pin, None) + config.change_pin(pin_to_int(''), pin_to_int(msg.pin), None) return Success(message='Device loaded') diff --git a/src/boot.py b/src/boot.py index e89d26aee2..12823984b1 100644 --- a/src/boot.py +++ b/src/boot.py @@ -1,4 +1,5 @@ from trezor import config +from trezor.pin import pin_to_int from trezor import loop from trezor import ui @@ -11,7 +12,7 @@ async def unlock_layout(): pin = await request_pin() else: pin = '' - if config.unlock(pin, show_pin_timeout): + if config.unlock(pin_to_int(pin), show_pin_timeout): return else: await unlock_failed() diff --git a/src/trezor/pin.py b/src/trezor/pin.py new file mode 100644 index 0000000000..f2e72341ae --- /dev/null +++ b/src/trezor/pin.py @@ -0,0 +1,6 @@ +from trezor import config +from trezor import log + + +def pin_to_int(pin): + return int('1'+pin)