feat(storage): Support PIN and wipe code of unlimited length.

storage/storage.c

@@ -35,33 +35,47 @@
 // The APP namespace which is reserved for storage related values.
 #define APP_STORAGE 0x00
 
-// Norcow storage key of the PIN entry log and PIN success log.
+// Norcow storage keys.
+// PIN entry log and PIN success log.
 #define PIN_LOGS_KEY ((APP_STORAGE << 8) | 0x01)
 
-// Norcow storage key of the combined salt, EDEK, ESAK and PIN verification code
+// Combined salt, EDEK, ESAK and PIN verification code entry.
-// entry.
 #define EDEK_PVC_KEY ((APP_STORAGE << 8) | 0x02)
 
-// Norcow storage key of the PIN set flag.
+// PIN set flag.
 #define PIN_NOT_SET_KEY ((APP_STORAGE << 8) | 0x03)
 
-// Norcow storage key of the storage version.
+// Authenticated storage version.
+// NOTE: This should equal the norcow version unless an upgrade is in progress.
 #define VERSION_KEY ((APP_STORAGE << 8) | 0x04)
 
-// Norcow storage key of the storage authentication tag.
+// Storage authentication tag.
 #define STORAGE_TAG_KEY ((APP_STORAGE << 8) | 0x05)
 
-// Norcow storage key of the wipe code data.
+// Wipe code data. Introduced in storage version 2.
 #define WIPE_CODE_DATA_KEY ((APP_STORAGE << 8) | 0x06)
 
-// Norcow storage key of the storage upgrade flag.
+// Storage upgrade flag. Introduced in storage version 2.
 #define STORAGE_UPGRADED_KEY ((APP_STORAGE << 8) | 0x07)
 
-// The PIN value corresponding to an invalid PIN.
+// Unauthenticated storage version. Introduced in storage version 3.
-#define PIN_INVALID 0
+// NOTE: This should always equal the value in VERSION_KEY.
+#define UNAUTH_VERSION_KEY ((APP_STORAGE << 8) | 0x08)
 
 // The PIN value corresponding to an empty PIN.
-#define PIN_EMPTY 1
+const uint8_t *PIN_EMPTY = (const uint8_t *)"";
+
+// The uint32 representation of an empty PIN, used prior to storage version 3.
+const uint32_t V0_PIN_EMPTY = 1;
+
+// Maximum number of PIN digits allowed prior to storage version 3.
+#define V0_MAX_PIN_LEN 9
+
+// Maximum length of the wipe code.
+// Some limit should be imposed on the length, because the wipe code takes up
+// storage space proportional to the length, as opposed to the PIN, which takes
+// up constant storage space.
+#define MAX_WIPE_CODE_LEN 50
 
 // Maximum number of failed unlock attempts.
 // NOTE: The PIN counter logic relies on this constant being less than or equal
@@ -114,23 +128,20 @@
 // The length of the ChaCha20 block in bytes.
 #define CHACHA20_BLOCK_SIZE 64
 
-// The length of the wipe code in bytes.
-#define WIPE_CODE_SIZE (sizeof(uint32_t))
-
 // The byte length of the salt used in checking the wipe code.
 #define WIPE_CODE_SALT_SIZE 8
 
 // The byte length of the tag used in checking the wipe code.
 #define WIPE_CODE_TAG_SIZE 8
 
-// The total length of the WIPE_CODE_DATA_KEY entry.
-#define WIPE_CODE_DATA_SIZE \
-  (WIPE_CODE_SIZE + WIPE_CODE_SALT_SIZE + WIPE_CODE_TAG_SIZE)
-
 // The value corresponding to an unconfigured wipe code.
-// NOTE: This is intentionally different from PIN_EMPTY so that we don't need
+// NOTE: This is intentionally different from an empty PIN so that we don't need
 // special handling when both the PIN and wipe code are not set.
-#define WIPE_CODE_EMPTY 0
+const uint8_t WIPE_CODE_EMPTY[] = {0, 0, 0, 0};
+#define WIPE_CODE_EMPTY_LEN 4
+
+// The uint32 representation of an empty wipe code used in storage version 2.
+#define V2_WIPE_CODE_EMPTY 0
 
 // The length of the counter tail in words.
 #define COUNTER_TAIL_WORDS 2
@@ -164,7 +175,10 @@ static void __handle_fault(const char *msg, const char *file, int line,
                            const char *func);
 #define handle_fault(msg) (__handle_fault(msg, __FILE__, __LINE__, __func__))
 
+static uint32_t pin_to_int(const uint8_t *pin, size_t pin_len);
 static secbool storage_upgrade(void);
+static secbool storage_upgrade_unlocked(const uint8_t *pin, size_t pin_len,
+                                        const uint8_t *ext_salt);
 static secbool storage_set_encrypted(const uint16_t key, const void *val,
                                      const uint16_t len);
 static secbool storage_get_encrypted(const uint16_t key, void *val_dest,
@@ -359,29 +373,53 @@ static secbool auth_get(uint16_t key, const void **val, uint16_t *len) {
   return sectrue;
 }
 
-static secbool set_wipe_code(uint32_t wipe_code) {
+static secbool set_wipe_code(const uint8_t *wipe_code, size_t wipe_code_len) {
-  if (wipe_code == PIN_EMPTY) {
+  if (wipe_code_len > MAX_WIPE_CODE_LEN ||
+      wipe_code_len > UINT16_MAX - WIPE_CODE_SALT_SIZE - WIPE_CODE_TAG_SIZE) {
+    return secfalse;
+  }
+
+  if (wipe_code_len == 0) {
     // This is to avoid having to check pin != PIN_EMPTY when checking the wipe
     // code.
     wipe_code = WIPE_CODE_EMPTY;
+    wipe_code_len = WIPE_CODE_EMPTY_LEN;
   }
 
   // The format of the WIPE_CODE_DATA_KEY entry is:
-  // wipe code (4 bytes), random salt (16 bytes), authentication tag (16 bytes)
+  // wipe code (variable), random salt (16 bytes), authentication tag (16 bytes)
-  // NOTE: We allocate extra space for the HMAC computation.
+  // NOTE: We allocate extra space for the HMAC result.
-  uint8_t wipe_code_data[WIPE_CODE_SIZE + WIPE_CODE_SALT_SIZE +
+  uint8_t salt_and_tag[WIPE_CODE_SALT_SIZE + SHA256_DIGEST_LENGTH] = {0};
-                         SHA256_DIGEST_LENGTH] = {0};
+  uint8_t *salt = salt_and_tag;
-  uint8_t *salt = wipe_code_data + WIPE_CODE_SIZE;
+  uint8_t *tag = salt_and_tag + WIPE_CODE_SALT_SIZE;
-  uint8_t *tag = wipe_code_data + WIPE_CODE_SIZE + WIPE_CODE_SALT_SIZE;
+
-  memcpy(wipe_code_data, &wipe_code, sizeof(wipe_code));
-  memzero(&wipe_code, sizeof(wipe_code));
   random_buffer(salt, WIPE_CODE_SALT_SIZE);
-  hmac_sha256(salt, WIPE_CODE_SALT_SIZE, wipe_code_data, WIPE_CODE_SIZE, tag);
+  hmac_sha256(salt, WIPE_CODE_SALT_SIZE, wipe_code, wipe_code_len, tag);
-  return norcow_set(WIPE_CODE_DATA_KEY, wipe_code_data, WIPE_CODE_DATA_SIZE);
+
+  // Preallocate the entry in the flash storage.
+  if (sectrue !=
+      norcow_set(WIPE_CODE_DATA_KEY, NULL,
+                 wipe_code_len + WIPE_CODE_SALT_SIZE + WIPE_CODE_TAG_SIZE)) {
+    return secfalse;
+  }
+
+  // Write wipe code into the preallocated entry.
+  if (sectrue !=
+      norcow_update_bytes(WIPE_CODE_DATA_KEY, 0, wipe_code, wipe_code_len)) {
+    return secfalse;
+  }
+
+  // Write salt and tag into the preallocated entry.
+  if (sectrue !=
+      norcow_update_bytes(WIPE_CODE_DATA_KEY, wipe_code_len, salt_and_tag,
+                          WIPE_CODE_SALT_SIZE + WIPE_CODE_TAG_SIZE)) {
+    return secfalse;
+  }
+
+  return sectrue;
 }
 
-static secbool is_not_wipe_code(uint32_t pin) {
+static secbool is_not_wipe_code(const uint8_t *pin, size_t pin_len) {
-  uint8_t wipe_code[WIPE_CODE_SIZE] = {0};
   uint8_t salt[WIPE_CODE_SALT_SIZE] = {0};
   uint8_t stored_tag[WIPE_CODE_TAG_SIZE] = {0};
   uint8_t computed_tag1[SHA256_DIGEST_LENGTH] = {0};
@@ -391,20 +429,20 @@ static secbool is_not_wipe_code(uint32_t pin) {
   const void *wipe_code_data = NULL;
   uint16_t len = 0;
   if (sectrue != norcow_get(WIPE_CODE_DATA_KEY, &wipe_code_data, &len) ||
-      len != WIPE_CODE_DATA_SIZE) {
+      len <= WIPE_CODE_SALT_SIZE + WIPE_CODE_TAG_SIZE) {
     handle_fault("no wipe code");
     return secfalse;
   }
-  memcpy(wipe_code, wipe_code_data, sizeof(wipe_code));
+  const uint8_t *wipe_code = (const uint8_t *)wipe_code_data;
-  memcpy(salt, (uint8_t *)wipe_code_data + WIPE_CODE_SIZE, sizeof(salt));
+  size_t wipe_code_len = len - WIPE_CODE_SALT_SIZE - WIPE_CODE_TAG_SIZE;
+  memcpy(salt, (uint8_t *)wipe_code_data + wipe_code_len, sizeof(salt));
   memcpy(stored_tag,
-         (uint8_t *)wipe_code_data + WIPE_CODE_SIZE + WIPE_CODE_SALT_SIZE,
+         (uint8_t *)wipe_code_data + wipe_code_len + WIPE_CODE_SALT_SIZE,
          sizeof(stored_tag));
 
   // Check integrity in case of flash read manipulation attack.
-  hmac_sha256(salt, WIPE_CODE_SALT_SIZE, wipe_code, sizeof(wipe_code),
+  hmac_sha256(salt, WIPE_CODE_SALT_SIZE, wipe_code, wipe_code_len,
               computed_tag1);
-  memzero(wipe_code, sizeof(wipe_code));
   if (sectrue != secequal(stored_tag, computed_tag1, sizeof(stored_tag))) {
     handle_fault("wipe code tag");
     return secfalse;
@@ -412,20 +450,17 @@ static secbool is_not_wipe_code(uint32_t pin) {
 
   // Prepare the authentication tag of the entered PIN.
   wait_random();
-  hmac_sha256(salt, WIPE_CODE_SALT_SIZE, (const uint8_t *)&pin, WIPE_CODE_SIZE,
+  hmac_sha256(salt, WIPE_CODE_SALT_SIZE, pin, pin_len, computed_tag1);
-              computed_tag1);
 
   // Recompute to check for fault injection attack.
   wait_random();
-  hmac_sha256(salt, WIPE_CODE_SALT_SIZE, (const uint8_t *)&pin, WIPE_CODE_SIZE,
+  hmac_sha256(salt, WIPE_CODE_SALT_SIZE, pin, pin_len, computed_tag2);
-              computed_tag2);
   memzero(salt, sizeof(salt));
   if (sectrue !=
       secequal(computed_tag1, computed_tag2, sizeof(computed_tag1))) {
     handle_fault("wipe code fault");
     return secfalse;
   }
-  memzero(&pin, sizeof(pin));
 
   // Compare wipe code with the entered PIN via the authentication tag.
   wait_random();
@@ -436,14 +471,10 @@ static secbool is_not_wipe_code(uint32_t pin) {
   return sectrue;
 }
 
-static void derive_kek(uint32_t pin, const uint8_t *random_salt,
+static void derive_kek(const uint8_t *pin, size_t pin_len,
-                       const uint8_t *ext_salt,
+                       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 + EXTERNAL_SALT_SIZE] = {
       0};
   size_t salt_len = 0;
@@ -465,8 +496,7 @@ static void derive_kek(uint32_t pin, const uint8_t *random_salt,
   }
 
   PBKDF2_HMAC_SHA256_CTX ctx = {0};
-  pbkdf2_hmac_sha256_Init(&ctx, (const uint8_t *)&pin, sizeof(pin), salt,
+  pbkdf2_hmac_sha256_Init(&ctx, pin, pin_len, salt, salt_len, 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) {
@@ -477,8 +507,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,
+  pbkdf2_hmac_sha256_Init(&ctx, pin, pin_len, salt, salt_len, 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) {
@@ -491,17 +520,11 @@ static void derive_kek(uint32_t pin, const uint8_t *random_salt,
 
   ui_rem -= DERIVE_SECS;
   memzero(&ctx, sizeof(PBKDF2_HMAC_SHA256_CTX));
-  memzero(&pin, sizeof(pin));
   memzero(&salt, sizeof(salt));
 }
 
-static secbool set_pin(uint32_t pin, const uint8_t *ext_salt) {
+static secbool set_pin(const uint8_t *pin, size_t pin_len,
-  // Fail if the PIN is the same as the wipe code. Ignore during upgrade.
+                       const uint8_t *ext_salt) {
-  if (norcow_active_version != 0 && sectrue != is_not_wipe_code(pin)) {
-    memzero(&pin, sizeof(pin));
-    return secfalse;
-  }
-
   // Encrypt the cached keys using the new PIN and set the new PVC.
   uint8_t buffer[RANDOM_SALT_SIZE + KEYS_SIZE + POLY1305_TAG_SIZE] = {0};
   uint8_t *rand_salt = buffer;
@@ -512,7 +535,7 @@ static secbool set_pin(uint32_t pin, const uint8_t *ext_salt) {
   uint8_t keiv[SHA256_DIGEST_LENGTH] = {0};
   chacha20poly1305_ctx ctx = {0};
   random_buffer(rand_salt, RANDOM_SALT_SIZE);
-  derive_kek(pin, rand_salt, ext_salt, kek, keiv);
+  derive_kek(pin, pin_len, rand_salt, ext_salt, kek, keiv);
   rfc7539_init(&ctx, kek, keiv);
   memzero(kek, sizeof(kek));
   memzero(keiv, sizeof(keiv));
@@ -524,14 +547,13 @@ static secbool set_pin(uint32_t pin, const uint8_t *ext_salt) {
   memzero(buffer, sizeof(buffer));
 
   if (ret == sectrue) {
-    if (pin == PIN_EMPTY) {
+    if (pin_len == 0) {
       ret = norcow_set(PIN_NOT_SET_KEY, &TRUE_BYTE, sizeof(TRUE_BYTE));
     } else {
       ret = norcow_set(PIN_NOT_SET_KEY, &FALSE_BYTE, sizeof(FALSE_BYTE));
     }
   }
 
-  memzero(&pin, sizeof(pin));
   return ret;
 }
 
@@ -637,15 +659,18 @@ static void init_wiped_storage(void) {
   ensure(auth_init(), "set_storage_auth_tag failed");
   ensure(storage_set_encrypted(VERSION_KEY, &version, sizeof(version)),
          "set_storage_version failed");
+  ensure(norcow_set(UNAUTH_VERSION_KEY, &version, sizeof(version)),
+         "set_unauth_storage_version failed");
   ensure(norcow_set(STORAGE_UPGRADED_KEY, &FALSE_WORD, sizeof(FALSE_WORD)),
          "set_storage_not_upgraded failed");
   ensure(pin_logs_init(0), "init_pin_logs failed");
-  ensure(set_wipe_code(WIPE_CODE_EMPTY), "set_wipe_code failed");
+  ensure(set_wipe_code(WIPE_CODE_EMPTY, WIPE_CODE_EMPTY_LEN),
+         "set_wipe_code failed");
 
   ui_total = DERIVE_SECS;
   ui_rem = ui_total;
   ui_message = PROCESSING_MSG;
-  ensure(set_pin(PIN_EMPTY, NULL), "init_pin failed");
+  ensure(set_pin(PIN_EMPTY, PIN_EMPTY_LEN, NULL), "init_pin failed");
 }
 
 void storage_init(PIN_UI_WAIT_CALLBACK callback, const uint8_t *salt,
@@ -861,6 +886,18 @@ void storage_lock(void) {
   memzero(authentication_sum, sizeof(authentication_sum));
 }
 
+// Returns the storage version that was used to lock the storage.
+static uint32_t get_lock_version(void) {
+  const void *val = NULL;
+  uint16_t len = 0;
+  if (sectrue != norcow_get(UNAUTH_VERSION_KEY, &val, &len) ||
+      len != sizeof(uint32_t)) {
+    handle_fault("no lock version");
+  }
+
+  return *(uint32_t *)val;
+}
+
 secbool check_storage_version(void) {
   uint32_t version = 0;
   uint16_t len = 0;
@@ -870,12 +907,19 @@ secbool check_storage_version(void) {
     handle_fault("storage version check");
     return secfalse;
   }
+
+  if (version != get_lock_version()) {
+    handle_fault("storage version check");
+    return secfalse;
+  }
+
   const void *storage_upgraded = NULL;
   if (sectrue != norcow_get(STORAGE_UPGRADED_KEY, &storage_upgraded, &len) ||
       len != sizeof(TRUE_WORD)) {
     handle_fault("storage version check");
     return secfalse;
   }
+
   if (version > norcow_active_version) {
     // Attack: Storage was downgraded.
     storage_wipe();
@@ -892,6 +936,8 @@ secbool check_storage_version(void) {
     norcow_set(STORAGE_UPGRADED_KEY, &FALSE_WORD, sizeof(FALSE_WORD));
     storage_set_encrypted(VERSION_KEY, &norcow_active_version,
                           sizeof(norcow_active_version));
+    norcow_set(UNAUTH_VERSION_KEY, &norcow_active_version,
+               sizeof(norcow_active_version));
   } else {
     // Standard operation. The storage was neither upgraded nor downgraded.
     if (*(const uint32_t *)storage_upgraded != FALSE_WORD) {
@@ -939,23 +985,41 @@ static secbool decrypt_dek(const uint8_t *kek, const uint8_t *keiv) {
   memcpy(cached_keys, keys, sizeof(keys));
   memzero(keys, sizeof(keys));
   memzero(tag, sizeof(tag));
-  // Check that the authenticated version number matches the norcow version.
+  return sectrue;
-  // NOTE: This also initializes the authentication_sum by calling
-  // storage_get_encrypted() which calls auth_get().
-  return check_storage_version();
 }
 
-static secbool unlock(uint32_t pin, const uint8_t *ext_salt) {
+static void ensure_not_wipe_code(const uint8_t *pin, size_t pin_len) {
-  if (sectrue != initialized || pin == PIN_INVALID) {
+  if (sectrue != is_not_wipe_code(pin, pin_len)) {
-    return secfalse;
+    storage_wipe();
+    error_shutdown("You have entered the", "wipe code. All private",
+                   "data has been erased.", NULL);
+  }
+}
+
+static secbool unlock(const uint8_t *pin, size_t pin_len,
+                      const uint8_t *ext_salt) {
+  const uint8_t *unlock_pin = pin;
+  size_t unlock_pin_len = pin_len;
+
+  // In case of an upgrade from version 1 or 2, encode the PIN to the old format
+  // and bump the total time of UI progress to account for the set_pin() call in
+  // storage_upgrade_unlocked().
+  uint32_t legacy_pin = 0;
+  if (get_lock_version() <= 2) {
+    ui_total += DERIVE_SECS;
+    ui_rem += DERIVE_SECS;
+    legacy_pin = pin_to_int(pin, pin_len);
+    unlock_pin = (const uint8_t *)&legacy_pin;
+    unlock_pin_len = sizeof(legacy_pin);
   }
 
-  storage_ensure_not_wipe_code(pin);
+  // Now we can check for wipe code.
+  ensure_not_wipe_code(unlock_pin, unlock_pin_len);
 
   // Get the pin failure counter
   uint32_t ctr = 0;
   if (sectrue != pin_get_fails(&ctr)) {
-    memzero(&pin, sizeof(pin));
+    memzero(&legacy_pin, sizeof(legacy_pin));
     return secfalse;
   }
 
@@ -981,6 +1045,7 @@ static secbool unlock(uint32_t pin, const uint8_t *ext_salt) {
           progress = ((ui_total - ui_rem) * 10 + i) * 100 / ui_total;
         }
         if (sectrue == ui_callback(ui_rem, progress, ui_message)) {
+          memzero(&legacy_pin, sizeof(legacy_pin));
           return secfalse;
         }
       }
@@ -995,14 +1060,15 @@ static secbool unlock(uint32_t pin, const uint8_t *ext_salt) {
   if (sectrue != initialized ||
       sectrue != norcow_get(EDEK_PVC_KEY, &rand_salt, &len) ||
       len != RANDOM_SALT_SIZE + KEYS_SIZE + PVC_SIZE) {
-    memzero(&pin, sizeof(pin));
+    memzero(&legacy_pin, sizeof(legacy_pin));
     handle_fault("no EDEK");
     return secfalse;
   }
   uint8_t kek[SHA256_DIGEST_LENGTH] = {0};
   uint8_t keiv[SHA256_DIGEST_LENGTH] = {0};
-  derive_kek(pin, (const uint8_t *)rand_salt, ext_salt, kek, keiv);
+  derive_kek(unlock_pin, unlock_pin_len, (const uint8_t *)rand_salt, ext_salt,
-  memzero(&pin, sizeof(pin));
+             kek, keiv);
+  memzero(&legacy_pin, sizeof(legacy_pin));
 
   // 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
@@ -1032,16 +1098,31 @@ static secbool unlock(uint32_t pin, const uint8_t *ext_salt) {
   memzero(kek, sizeof(kek));
   memzero(keiv, sizeof(keiv));
 
+  // Check for storage upgrades that need to be performed after unlocking and
+  // check that the authenticated version number matches the unauthenticated
+  // version and norcow version.
+  // NOTE: This also initializes the authentication_sum by calling
+  // storage_get_encrypted() which calls auth_get().
+  if (sectrue != storage_upgrade_unlocked(pin, pin_len, ext_salt) ||
+      sectrue != check_storage_version()) {
+    return secfalse;
+  }
+
   unlocked = sectrue;
 
   // Finally set the counter to 0 to indicate success.
   return pin_fails_reset();
 }
 
-secbool storage_unlock(uint32_t pin, const uint8_t *ext_salt) {
+secbool storage_unlock(const uint8_t *pin, size_t pin_len,
+                       const uint8_t *ext_salt) {
+  if (sectrue != initialized || pin == NULL) {
+    return secfalse;
+  }
+
   ui_total = DERIVE_SECS;
   ui_rem = ui_total;
-  if (pin == PIN_EMPTY) {
+  if (pin_len == 0) {
     if (ui_message == NULL) {
       ui_message = STARTING_MSG;
     } else {
@@ -1050,7 +1131,7 @@ secbool storage_unlock(uint32_t pin, const uint8_t *ext_salt) {
   } else {
     ui_message = VERIFYING_PIN_MSG;
   }
-  return unlock(pin, ext_salt);
+  return unlock(pin, pin_len, ext_salt);
 }
 
 /*
@@ -1317,34 +1398,43 @@ 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(const uint8_t *oldpin, size_t oldpin_len,
+                           const uint8_t *newpin, size_t newpin_len,
                            const uint8_t *old_ext_salt,
                            const uint8_t *new_ext_salt) {
-  if (sectrue != initialized || oldpin == PIN_INVALID ||
+  if (sectrue != initialized || oldpin == NULL || newpin == NULL) {
-      newpin == PIN_INVALID) {
     return secfalse;
   }
 
   ui_total = 2 * DERIVE_SECS;
   ui_rem = ui_total;
-  ui_message = (oldpin != PIN_EMPTY && newpin == PIN_EMPTY) ? VERIFYING_PIN_MSG
+  ui_message =
-                                                            : PROCESSING_MSG;
+      (oldpin_len != 0 && newpin_len == 0) ? VERIFYING_PIN_MSG : PROCESSING_MSG;
 
-  if (sectrue != unlock(oldpin, old_ext_salt)) {
+  if (sectrue != unlock(oldpin, oldpin_len, old_ext_salt)) {
     return secfalse;
   }
-  secbool ret = set_pin(newpin, new_ext_salt);
+
-  memzero(&oldpin, sizeof(oldpin));
+  // Fail if the new PIN is the same as the wipe code.
-  memzero(&newpin, sizeof(newpin));
+  if (sectrue != is_not_wipe_code(newpin, newpin_len)) {
-  return ret;
+    return secfalse;
+  }
+
+  return set_pin(newpin, newpin_len, new_ext_salt);
 }
 
-void storage_ensure_not_wipe_code(uint32_t pin) {
+void storage_ensure_not_wipe_code(const uint8_t *pin, size_t pin_len) {
-  if (sectrue != is_not_wipe_code(pin)) {
+  // If we are unlocking the storage during upgrade from version 2 or lower,
-    storage_wipe();
+  // then encode the PIN to the old format.
-    error_shutdown("You have entered the", "wipe code. All private",
+  uint32_t legacy_pin = 0;
-                   "data has been erased.", NULL);
+  if (get_lock_version() <= 2) {
+    legacy_pin = pin_to_int(pin, pin_len);
+    pin = (const uint8_t *)&legacy_pin;
+    pin_len = sizeof(legacy_pin);
   }
+
+  ensure_not_wipe_code(pin, pin_len);
+  memzero(&legacy_pin, sizeof(legacy_pin));
 }
 
 secbool storage_has_wipe_code(void) {
@@ -1352,29 +1442,28 @@ secbool storage_has_wipe_code(void) {
     return secfalse;
   }
 
-  return is_not_wipe_code(WIPE_CODE_EMPTY);
+  return is_not_wipe_code(WIPE_CODE_EMPTY, WIPE_CODE_EMPTY_LEN);
 }
 
-secbool storage_change_wipe_code(uint32_t pin, const uint8_t *ext_salt,
+secbool storage_change_wipe_code(const uint8_t *pin, size_t pin_len,
-                                 uint32_t wipe_code) {
+                                 const uint8_t *ext_salt,
-  if (sectrue != initialized || (pin != PIN_EMPTY && pin == wipe_code) ||
+                                 const uint8_t *wipe_code,
-      pin == PIN_INVALID || wipe_code == PIN_INVALID) {
+                                 size_t wipe_code_len) {
-    memzero(&pin, sizeof(pin));
+  if (sectrue != initialized || pin == NULL || wipe_code == NULL ||
-    memzero(&wipe_code, sizeof(wipe_code));
+      (pin_len != 0 && pin_len == wipe_code_len &&
+       memcmp(pin, wipe_code, pin_len) == 0)) {
     return secfalse;
   }
 
   ui_total = DERIVE_SECS;
   ui_rem = ui_total;
-  ui_message = (pin != PIN_EMPTY && wipe_code == PIN_EMPTY) ? VERIFYING_PIN_MSG
+  ui_message =
-                                                            : PROCESSING_MSG;
+      (pin_len != 0 && wipe_code_len == 0) ? VERIFYING_PIN_MSG : PROCESSING_MSG;
 
   secbool ret = secfalse;
-  if (sectrue == unlock(pin, ext_salt)) {
+  if (sectrue == unlock(pin, pin_len, ext_salt)) {
-    ret = set_wipe_code(wipe_code);
+    ret = set_wipe_code(wipe_code, wipe_code_len);
   }
-  memzero(&pin, sizeof(pin));
-  memzero(&wipe_code, sizeof(wipe_code));
   return ret;
 }
 
@@ -1450,7 +1539,59 @@ static secbool v0_pin_get_fails(uint32_t *ctr) {
   return sectrue;
 }
 
+// Legacy conversion of PIN to the uint32 scheme that was used prior to storage
+// version 3.
+static uint32_t pin_to_int(const uint8_t *pin, size_t pin_len) {
+  if (pin_len > V0_MAX_PIN_LEN) {
+    return 0;
+  }
+
+  uint32_t val = 1;
+  size_t i = 0;
+  for (i = 0; i < pin_len; ++i) {
+    if (pin[i] < '0' || pin[i] > '9') {
+      return 0;
+    }
+    val = 10 * val + pin[i] - '0';
+  }
+
+  return val;
+}
+
+// Legacy conversion of wipe code from the uint32 scheme that was used prior to
+// storage version 3.
+static char *int_to_wipe_code(uint32_t val) {
+  static char wipe_code[V0_MAX_PIN_LEN + 1] = {0};
+  size_t pos = sizeof(wipe_code) - 1;
+  wipe_code[pos] = '\0';
+
+  // Handle the special representation of an empty wipe code.
+  if (val == V2_WIPE_CODE_EMPTY) {
+    return &wipe_code[pos];
+  }
+
+  if (val == V0_PIN_EMPTY) {
+    return NULL;
+  }
+
+  // Convert a non-empty wipe code.
+  while (val != 1) {
+    if (pos == 0) {
+      return NULL;
+    }
+    pos--;
+    wipe_code[pos] = '0' + (val % 10);
+    val /= 10;
+  }
+  return &wipe_code[pos];
+}
+
 static secbool storage_upgrade(void) {
+  // Storage version 0: plaintext norcow
+  // Storage version 1: encrypted norcow
+  // Storage version 2: adds 9 digit wipe code
+  // Storage version 3: adds variable length PIN and wipe code
+
   const uint16_t V0_PIN_KEY = 0x0000;
   const uint16_t V0_PIN_FAIL_KEY = 0x0001;
   uint16_t key = 0;
@@ -1475,10 +1616,12 @@ static secbool storage_upgrade(void) {
     ui_total = DERIVE_SECS;
     ui_rem = ui_total;
     ui_message = PROCESSING_MSG;
-    if (sectrue == norcow_get(V0_PIN_KEY, &val, &len)) {
+    secbool found = norcow_get(V0_PIN_KEY, &val, &len);
-      set_pin(*(const uint32_t *)val, NULL);
+    if (sectrue == found && *(const uint32_t *)val != V0_PIN_EMPTY) {
+      set_pin((const uint8_t *)val, len, NULL);
     } else {
-      set_pin(PIN_EMPTY, NULL);
+      set_pin((const uint8_t *)&V0_PIN_EMPTY, sizeof(V0_PIN_EMPTY), NULL);
+      ret = norcow_set(PIN_NOT_SET_KEY, &TRUE_BYTE, sizeof(TRUE_BYTE));
     }
 
     // Convert PIN failure counter.
@@ -1516,17 +1659,75 @@ static secbool storage_upgrade(void) {
     }
   }
 
+  // Set wipe code.
   if (norcow_active_version <= 1) {
-    if (sectrue != set_wipe_code(WIPE_CODE_EMPTY)) {
+    if (sectrue != set_wipe_code(WIPE_CODE_EMPTY, WIPE_CODE_EMPTY_LEN)) {
       return secfalse;
     }
   }
 
-  if (sectrue !=
+  if (norcow_active_version <= 2) {
-      norcow_set(STORAGE_UPGRADED_KEY, &TRUE_WORD, sizeof(TRUE_WORD))) {
+    // Set UNAUTH_VERSION_KEY, so that it matches VERSION_KEY.
-    return secfalse;
+    uint32_t version = 1;
+
+    // The storage may have gone through an upgrade to version 2 without having
+    // been unlocked. We can tell by looking at STORAGE_UPGRADED_KEY.
+    if (sectrue == norcow_get(STORAGE_UPGRADED_KEY, &val, &len) &&
+        len == sizeof(FALSE_WORD) && *((uint32_t *)val) == FALSE_WORD) {
+      version = 2;
+    }
+
+    if (sectrue != norcow_set(UNAUTH_VERSION_KEY, &version, sizeof(version))) {
+      return secfalse;
+    }
   }
 
+  norcow_set(STORAGE_UPGRADED_KEY, &TRUE_WORD, sizeof(TRUE_WORD));
+
   norcow_active_version = NORCOW_VERSION;
   return norcow_upgrade_finish();
 }
+
+static secbool storage_upgrade_unlocked(const uint8_t *pin, size_t pin_len,
+                                        const uint8_t *ext_salt) {
+  uint32_t version = 0;
+  uint16_t len = 0;
+  if (sectrue !=
+          storage_get_encrypted(VERSION_KEY, &version, sizeof(version), &len) ||
+      len != sizeof(version)) {
+    handle_fault("storage version check");
+    return secfalse;
+  }
+
+  secbool ret = sectrue;
+  if (version <= 2) {
+    // Upgrade EDEK_PVC_KEY from the old uint32 PIN scheme to the new
+    // variable-length PIN scheme.
+    if (sectrue != set_pin(pin, pin_len, ext_salt)) {
+      return secfalse;
+    }
+  }
+
+  if (version == 2) {
+    // Upgrade WIPE_CODE_DATA_KEY from the old uint32 scheme to the new
+    // variable-length scheme.
+    const void *wipe_code_data = NULL;
+    if (sectrue != norcow_get(WIPE_CODE_DATA_KEY, &wipe_code_data, &len) ||
+        len < sizeof(uint32_t)) {
+      handle_fault("no wipe code");
+      return secfalse;
+    }
+
+    char *wipe_code = int_to_wipe_code(*(uint32_t *)wipe_code_data);
+    if (wipe_code == NULL) {
+      handle_fault("invalid wipe code");
+      return secfalse;
+    }
+
+    size_t wipe_code_len = strnlen(wipe_code, V0_MAX_PIN_LEN);
+    ret = set_wipe_code((const uint8_t *)wipe_code, wipe_code_len);
+    memzero(wipe_code, wipe_code_len);
+  }
+
+  return ret;
+}

storage/storage.h

@@ -37,6 +37,10 @@
 // Mask for extracting the "real" app_id.
 #define FLAGS_APPID 0x3F
 
+// The PIN value corresponding to an empty PIN.
+extern const uint8_t *PIN_EMPTY;
+#define PIN_EMPTY_LEN 0
+
 typedef secbool (*PIN_UI_WAIT_CALLBACK)(uint32_t wait, uint32_t progress,
                                         const char *message);
 
@@ -45,17 +49,21 @@ 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(uint32_t pin, const uint8_t *ext_salt);
+secbool storage_unlock(const uint8_t *pin, size_t pin_len,
+                       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(uint32_t oldpin, uint32_t newpin,
+secbool storage_change_pin(const uint8_t *oldpin, size_t oldpin_len,
+                           const uint8_t *newpin, size_t newpin_len,
                            const uint8_t *old_ext_salt,
                            const uint8_t *new_ext_salt);
-void storage_ensure_not_wipe_code(uint32_t pin);
+void storage_ensure_not_wipe_code(const uint8_t *pin, size_t pin_len);
 secbool storage_has_wipe_code(void);
-secbool storage_change_wipe_code(uint32_t pin, const uint8_t *ext_salt,
+secbool storage_change_wipe_code(const uint8_t *pin, size_t pin_len,
-                                 uint32_t wipe_code);
+                                 const uint8_t *ext_salt,
+                                 const uint8_t *wipe_code,
+                                 size_t wipe_code_len);
 secbool storage_has(const uint16_t key);
 secbool storage_get(const uint16_t key, void *val, const uint16_t max_len,
                     uint16_t *len);

storage/tests/c/norcow_config.h

@@ -42,6 +42,6 @@
 /*
  * Current storage version.
  */
-#define NORCOW_VERSION ((uint32_t)0x00000002)
+#define NORCOW_VERSION ((uint32_t)0x00000003)
 
 #endif