diff --git a/core/embed/extmod/modtrezorconfig/modtrezorconfig.c b/core/embed/extmod/modtrezorconfig/modtrezorconfig.c index 43572e78b..3c6356883 100644 --- a/core/embed/extmod/modtrezorconfig/modtrezorconfig.c +++ b/core/embed/extmod/modtrezorconfig/modtrezorconfig.c @@ -67,30 +67,41 @@ STATIC mp_obj_t mod_trezorconfig_init(size_t n_args, const mp_obj_t *args) { STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mod_trezorconfig_init_obj, 0, 1, mod_trezorconfig_init); -/// def unlock(pin: int) -> bool: +/// def unlock(pin: int, ext_salt: Optional[bytes] = None) -> bool: /// """ -/// Attempts to unlock the storage with given PIN. Returns True on -/// success, False on failure. +/// Attempts to unlock the storage with the given PIN and external salt. +/// Returns True on success, False on failure. /// """ -STATIC mp_obj_t mod_trezorconfig_unlock(mp_obj_t pin) { - uint32_t pin_i = trezor_obj_get_uint(pin); - if (sectrue != storage_unlock(pin_i)) { +STATIC mp_obj_t mod_trezorconfig_unlock(size_t n_args, const mp_obj_t *args) { + uint32_t pin = trezor_obj_get_uint(args[0]); + const uint8_t *ext_salt = NULL; + if (n_args > 1 && args[1] != mp_const_none) { + mp_buffer_info_t ext_salt_b; + mp_get_buffer_raise(args[1], &ext_salt_b, MP_BUFFER_READ); + if (ext_salt_b.len != EXTERNAL_SALT_SIZE) + mp_raise_msg(&mp_type_ValueError, "Invalid length of external salt."); + ext_salt = ext_salt_b.buf; + } + + if (sectrue != storage_unlock(pin, ext_salt)) { 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_VAR_BETWEEN(mod_trezorconfig_unlock_obj, 1, 2, + mod_trezorconfig_unlock); -/// def check_pin(pin: int) -> bool: +/// def check_pin(pin: int, ext_salt: Optional[bytes] = None) -> bool: /// """ -/// Check the given PIN. Returns True on success, False on failure. +/// Check the given PIN with the given external salt. +/// Returns True on success, False on failure. /// """ -STATIC mp_obj_t mod_trezorconfig_check_pin(mp_obj_t pin) { - return mod_trezorconfig_unlock(pin); +STATIC mp_obj_t mod_trezorconfig_check_pin(size_t n_args, + const mp_obj_t *args) { + return mod_trezorconfig_unlock(n_args, args); } -STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_trezorconfig_check_pin_obj, - mod_trezorconfig_check_pin); +STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mod_trezorconfig_check_pin_obj, 1, 2, + mod_trezorconfig_check_pin); /// def lock() -> None: /// """ @@ -126,20 +137,43 @@ STATIC mp_obj_t mod_trezorconfig_get_pin_rem(void) { STATIC MP_DEFINE_CONST_FUN_OBJ_0(mod_trezorconfig_get_pin_rem_obj, mod_trezorconfig_get_pin_rem); -/// def change_pin(pin: int, newpin: int) -> bool: +/// def change_pin( +/// oldpin: int, +/// newpin: int, +/// old_ext_salt: Optional[bytes] = None, +/// new_ext_salt: Optional[bytes] = None, +/// ) -> bool: /// """ -/// Change PIN. Returns True on success, False on failure. +/// Change PIN and external salt. Returns True on success, False on failure. /// """ -STATIC mp_obj_t mod_trezorconfig_change_pin(mp_obj_t pin, mp_obj_t newpin) { - uint32_t pin_i = trezor_obj_get_uint(pin); - uint32_t newpin_i = trezor_obj_get_uint(newpin); - if (sectrue != storage_change_pin(pin_i, newpin_i)) { +STATIC mp_obj_t mod_trezorconfig_change_pin(size_t n_args, + const mp_obj_t *args) { + uint32_t oldpin = trezor_obj_get_uint(args[0]); + uint32_t newpin = trezor_obj_get_uint(args[1]); + mp_buffer_info_t ext_salt_b; + const uint8_t *old_ext_salt = NULL; + if (n_args > 2 && args[2] != mp_const_none) { + mp_get_buffer_raise(args[2], &ext_salt_b, MP_BUFFER_READ); + if (ext_salt_b.len != EXTERNAL_SALT_SIZE) + mp_raise_msg(&mp_type_ValueError, "Invalid length of external salt."); + old_ext_salt = ext_salt_b.buf; + } + const uint8_t *new_ext_salt = NULL; + if (n_args > 3 && args[3] != mp_const_none) { + mp_get_buffer_raise(args[3], &ext_salt_b, MP_BUFFER_READ); + if (ext_salt_b.len != EXTERNAL_SALT_SIZE) + mp_raise_msg(&mp_type_ValueError, "Invalid length of external salt."); + new_ext_salt = ext_salt_b.buf; + } + + if (sectrue != + storage_change_pin(oldpin, newpin, old_ext_salt, new_ext_salt)) { 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_VAR_BETWEEN(mod_trezorconfig_change_pin_obj, 2, + 4, mod_trezorconfig_change_pin); /// def get(app: int, key: int, public: bool = False) -> Optional[bytes]: /// """ diff --git a/core/mocks/generated/trezorconfig.pyi b/core/mocks/generated/trezorconfig.pyi index 399ea4d95..ae3742467 100644 --- a/core/mocks/generated/trezorconfig.pyi +++ b/core/mocks/generated/trezorconfig.pyi @@ -12,17 +12,18 @@ def init( # extmod/modtrezorconfig/modtrezorconfig.c -def unlock(pin: int) -> bool: +def unlock(pin: int, ext_salt: Optional[bytes] = None) -> bool: """ - Attempts to unlock the storage with given PIN. Returns True on - success, False on failure. + Attempts to unlock the storage with the given PIN and external salt. + Returns True on success, False on failure. """ # extmod/modtrezorconfig/modtrezorconfig.c -def check_pin(pin: int) -> bool: +def check_pin(pin: int, ext_salt: Optional[bytes] = None) -> bool: """ - Check the given PIN. Returns True on success, False on failure. + Check the given PIN with the given external salt. + Returns True on success, False on failure. """ @@ -48,9 +49,14 @@ def get_pin_rem() -> int: # extmod/modtrezorconfig/modtrezorconfig.c -def change_pin(pin: int, newpin: int) -> bool: +def change_pin( + oldpin: int, + newpin: int, + old_ext_salt: Optional[bytes] = None, + new_ext_salt: Optional[bytes] = None, +) -> bool: """ - Change PIN. Returns True on success, False on failure. + Change PIN and external salt. Returns True on success, False on failure. """ diff --git a/legacy/firmware/config.c b/legacy/firmware/config.c index 16a206d46..6272cff0c 100644 --- a/legacy/firmware/config.c +++ b/legacy/firmware/config.c @@ -316,9 +316,9 @@ static secbool config_upgrade_v10(void) { } storage_init(NULL, HW_ENTROPY_DATA, HW_ENTROPY_LEN); - storage_unlock(PIN_EMPTY); + storage_unlock(PIN_EMPTY, NULL); if (config.has_pin) { - storage_change_pin(PIN_EMPTY, pin_to_int(config.pin)); + storage_change_pin(PIN_EMPTY, pin_to_int(config.pin), NULL, NULL); } while (pin_wait != 0) { @@ -386,7 +386,7 @@ void config_init(void) { // Auto-unlock storage if no PIN is set. if (storage_is_unlocked() == secfalse && storage_has_pin() == secfalse) { - storage_unlock(PIN_EMPTY); + storage_unlock(PIN_EMPTY, NULL); } uint16_t len = 0; @@ -759,7 +759,7 @@ bool config_containsMnemonic(const char *mnemonic) { */ bool config_unlock(const char *pin) { char oldTiny = usbTiny(1); - secbool ret = storage_unlock(pin_to_int(pin)); + secbool ret = storage_unlock(pin_to_int(pin), NULL); usbTiny(oldTiny); return sectrue == ret; } @@ -773,7 +773,8 @@ bool config_changePin(const char *old_pin, const char *new_pin) { } char oldTiny = usbTiny(1); - secbool ret = storage_change_pin(pin_to_int(old_pin), new_pin_int); + secbool ret = + storage_change_pin(pin_to_int(old_pin), new_pin_int, NULL, NULL); usbTiny(oldTiny); #if DEBUG_LINK @@ -925,7 +926,7 @@ void config_wipe(void) { char oldTiny = usbTiny(1); storage_wipe(); if (storage_is_unlocked() != sectrue) { - storage_unlock(PIN_EMPTY); + storage_unlock(PIN_EMPTY, NULL); } usbTiny(oldTiny); random_buffer((uint8_t *)config_uuid, sizeof(config_uuid)); diff --git a/storage/storage.c b/storage/storage.c index 31c10898b..50ef07e01 100644 --- a/storage/storage.c +++ b/storage/storage.c @@ -331,15 +331,26 @@ static secbool auth_get(uint16_t key, const void **val, uint16_t *len) { } static void derive_kek(uint32_t pin, const uint8_t *random_salt, + const uint8_t *ext_salt, uint8_t kek[SHA256_DIGEST_LENGTH], uint8_t keiv[SHA256_DIGEST_LENGTH]) { #if BYTE_ORDER == BIG_ENDIAN REVERSE32(pin, pin); #endif - uint8_t salt[HARDWARE_SALT_SIZE + RANDOM_SALT_SIZE]; - memcpy(salt, hardware_salt, HARDWARE_SALT_SIZE); - memcpy(salt + HARDWARE_SALT_SIZE, random_salt, RANDOM_SALT_SIZE); + uint8_t salt[HARDWARE_SALT_SIZE + RANDOM_SALT_SIZE + EXTERNAL_SALT_SIZE]; + size_t salt_len = 0; + + memcpy(salt + salt_len, hardware_salt, HARDWARE_SALT_SIZE); + salt_len += HARDWARE_SALT_SIZE; + + memcpy(salt + salt_len, random_salt, RANDOM_SALT_SIZE); + salt_len += RANDOM_SALT_SIZE; + + if (ext_salt != NULL) { + memcpy(salt + salt_len, ext_salt, EXTERNAL_SALT_SIZE); + salt_len += EXTERNAL_SALT_SIZE; + } uint32_t progress = (ui_total - ui_rem) * 1000 / ui_total; if (ui_callback && ui_message) { @@ -348,7 +359,7 @@ static void derive_kek(uint32_t pin, const uint8_t *random_salt, PBKDF2_HMAC_SHA256_CTX ctx; pbkdf2_hmac_sha256_Init(&ctx, (const uint8_t *)&pin, sizeof(pin), salt, - sizeof(salt), 1); + salt_len, 1); for (int i = 1; i <= 5; i++) { pbkdf2_hmac_sha256_Update(&ctx, PIN_ITER_COUNT / 10); if (ui_callback && ui_message) { @@ -360,7 +371,7 @@ static void derive_kek(uint32_t pin, const uint8_t *random_salt, pbkdf2_hmac_sha256_Final(&ctx, kek); pbkdf2_hmac_sha256_Init(&ctx, (const uint8_t *)&pin, sizeof(pin), salt, - sizeof(salt), 2); + salt_len, 2); for (int i = 6; i <= 10; i++) { pbkdf2_hmac_sha256_Update(&ctx, PIN_ITER_COUNT / 10); if (ui_callback && ui_message) { @@ -377,17 +388,17 @@ static void derive_kek(uint32_t pin, const uint8_t *random_salt, memzero(&salt, sizeof(salt)); } -static secbool set_pin(uint32_t pin) { +static secbool set_pin(uint32_t pin, const uint8_t *ext_salt) { uint8_t buffer[RANDOM_SALT_SIZE + KEYS_SIZE + POLY1305_TAG_SIZE]; - uint8_t *salt = buffer; + uint8_t *rand_salt = buffer; uint8_t *ekeys = buffer + RANDOM_SALT_SIZE; uint8_t *pvc = buffer + RANDOM_SALT_SIZE + KEYS_SIZE; uint8_t kek[SHA256_DIGEST_LENGTH]; uint8_t keiv[SHA256_DIGEST_LENGTH]; chacha20poly1305_ctx ctx; - random_buffer(salt, RANDOM_SALT_SIZE); - derive_kek(pin, salt, kek, keiv); + random_buffer(rand_salt, RANDOM_SALT_SIZE); + derive_kek(pin, rand_salt, ext_salt, kek, keiv); rfc7539_init(&ctx, kek, keiv); memzero(kek, sizeof(kek)); memzero(keiv, sizeof(keiv)); @@ -515,7 +526,7 @@ static void init_wiped_storage(void) { ui_total = DERIVE_SECS; ui_rem = ui_total; ui_message = PROCESSING_MSG; - ensure(set_pin(PIN_EMPTY), "init_pin failed"); + ensure(set_pin(PIN_EMPTY, NULL), "init_pin failed"); if (unlocked != sectrue) { memzero(cached_keys, sizeof(cached_keys)); } @@ -784,7 +795,7 @@ static secbool decrypt_dek(const uint8_t *kek, const uint8_t *keiv) { return sectrue; } -static secbool unlock(uint32_t pin) { +static secbool unlock(uint32_t pin, const uint8_t *ext_salt) { if (sectrue != initialized) { return secfalse; } @@ -827,10 +838,10 @@ static secbool unlock(uint32_t pin) { // Read the random salt from EDEK_PVC_KEY and use it to derive the KEK and // KEIV from the PIN. - const void *salt = NULL; + const void *rand_salt = NULL; uint16_t len = 0; if (sectrue != initialized || - sectrue != norcow_get(EDEK_PVC_KEY, &salt, &len) || + sectrue != norcow_get(EDEK_PVC_KEY, &rand_salt, &len) || len != RANDOM_SALT_SIZE + KEYS_SIZE + PVC_SIZE) { memzero(&pin, sizeof(pin)); handle_fault("no EDEK"); @@ -838,7 +849,7 @@ static secbool unlock(uint32_t pin) { } uint8_t kek[SHA256_DIGEST_LENGTH]; uint8_t keiv[SHA256_DIGEST_LENGTH]; - derive_kek(pin, (const uint8_t *)salt, kek, keiv); + derive_kek(pin, (const uint8_t *)rand_salt, ext_salt, kek, keiv); memzero(&pin, sizeof(pin)); // First, we increase PIN fail counter in storage, even before checking the @@ -875,7 +886,7 @@ static secbool unlock(uint32_t pin) { return pin_fails_reset(); } -secbool storage_unlock(uint32_t pin) { +secbool storage_unlock(uint32_t pin, const uint8_t *ext_salt) { ui_total = DERIVE_SECS; ui_rem = ui_total; if (pin == PIN_EMPTY) { @@ -887,7 +898,7 @@ secbool storage_unlock(uint32_t pin) { } else { ui_message = VERIFYING_PIN_MSG; } - return unlock(pin); + return unlock(pin, ext_salt); } /* @@ -1152,7 +1163,9 @@ uint32_t storage_get_pin_rem(void) { return PIN_MAX_TRIES - ctr; } -secbool storage_change_pin(uint32_t oldpin, uint32_t newpin) { +secbool storage_change_pin(uint32_t oldpin, uint32_t newpin, + const uint8_t *old_ext_salt, + const uint8_t *new_ext_salt) { if (sectrue != initialized) { return secfalse; } @@ -1162,10 +1175,10 @@ secbool storage_change_pin(uint32_t oldpin, uint32_t newpin) { ui_message = (oldpin != PIN_EMPTY && newpin == PIN_EMPTY) ? VERIFYING_PIN_MSG : PROCESSING_MSG; - if (sectrue != unlock(oldpin)) { + if (sectrue != unlock(oldpin, old_ext_salt)) { return secfalse; } - secbool ret = set_pin(newpin); + secbool ret = set_pin(newpin, new_ext_salt); memzero(&oldpin, sizeof(oldpin)); memzero(&newpin, sizeof(newpin)); return ret; @@ -1268,9 +1281,9 @@ static secbool storage_upgrade(void) { ui_rem = ui_total; ui_message = PROCESSING_MSG; if (sectrue == norcow_get(V0_PIN_KEY, &val, &len)) { - set_pin(*(const uint32_t *)val); + set_pin(*(const uint32_t *)val, NULL); } else { - set_pin(PIN_EMPTY); + set_pin(PIN_EMPTY, NULL); } // Convert PIN failure counter. diff --git a/storage/storage.h b/storage/storage.h index 2abb409cc..2a8b22375 100644 --- a/storage/storage.h +++ b/storage/storage.h @@ -24,6 +24,9 @@ #include #include "secbool.h" +// The length of the external salt in bytes. +#define EXTERNAL_SALT_SIZE 32 + typedef secbool (*PIN_UI_WAIT_CALLBACK)(uint32_t wait, uint32_t progress, const char *message); @@ -32,11 +35,13 @@ void storage_init(PIN_UI_WAIT_CALLBACK callback, const uint8_t *salt, void storage_wipe(void); secbool storage_is_unlocked(void); void storage_lock(void); -secbool storage_unlock(const uint32_t pin); +secbool storage_unlock(const uint32_t pin, const uint8_t *ext_salt); secbool storage_has_pin(void); secbool storage_pin_fails_increase(void); uint32_t storage_get_pin_rem(void); -secbool storage_change_pin(const uint32_t oldpin, const uint32_t newpin); +secbool storage_change_pin(const uint32_t oldpin, const uint32_t newpin, + const uint8_t *old_ext_salt, + const uint8_t *new_ext_salt); secbool storage_get(const uint16_t key, void *val, const uint16_t max_len, uint16_t *len); secbool storage_set(const uint16_t key, const void *val, uint16_t len); diff --git a/storage/tests/c/libtrezor-storage.so b/storage/tests/c/libtrezor-storage.so index 83aa07284..64898cc47 100755 Binary files a/storage/tests/c/libtrezor-storage.so and b/storage/tests/c/libtrezor-storage.so differ diff --git a/storage/tests/c/storage.py b/storage/tests/c/storage.py index d446b0dcf..51ba59498 100644 --- a/storage/tests/c/storage.py +++ b/storage/tests/c/storage.py @@ -1,6 +1,7 @@ import ctypes as c import os +EXTERNAL_SALT_LEN = 32 sectrue = -1431655766 # 0xAAAAAAAAA fname = os.path.join(os.path.dirname(__file__), "libtrezor-storage.so") @@ -20,8 +21,10 @@ class Storage: def wipe(self) -> None: self.lib.storage_wipe() - def unlock(self, pin: int) -> bool: - return sectrue == self.lib.storage_unlock(c.c_uint32(pin)) + def unlock(self, pin: int, ext_salt: bytes = None) -> bool: + if ext_salt is not None and len(ext_salt) != EXTERNAL_SALT_LEN: + raise ValueError + return sectrue == self.lib.storage_unlock(c.c_uint32(pin), ext_salt) def lock(self) -> None: self.lib.storage_lock() @@ -32,9 +35,19 @@ class Storage: def get_pin_rem(self) -> int: return self.lib.storage_get_pin_rem() - def change_pin(self, oldpin: int, newpin: int) -> bool: + def change_pin( + self, + oldpin: int, + newpin: int, + old_ext_salt: bytes = None, + new_ext_salt: bytes = None, + ) -> bool: + if old_ext_salt is not None and len(old_ext_salt) != EXTERNAL_SALT_LEN: + raise ValueError + if new_ext_salt is not None and len(new_ext_salt) != EXTERNAL_SALT_LEN: + raise ValueError return sectrue == self.lib.storage_change_pin( - c.c_uint32(oldpin), c.c_uint32(newpin) + c.c_uint32(oldpin), c.c_uint32(newpin), old_ext_salt, new_ext_salt ) def get(self, key: int) -> bytes: diff --git a/storage/tests/c0/libtrezor-storage0.so b/storage/tests/c0/libtrezor-storage0.so index e03da87a7..3504b9d60 100755 Binary files a/storage/tests/c0/libtrezor-storage0.so and b/storage/tests/c0/libtrezor-storage0.so differ