storage: Add external salt parameter to unlock() and change_pin().

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]:
 ///     """

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.
     """
 
 

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));

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.

storage/storage.h

@@ -24,6 +24,9 @@
 #include <stdint.h>
 #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);

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: