Skip to content

Commit

Permalink
Fix storage upgrade with PIN
Browse files Browse the repository at this point in the history
Missed this case because even DEBUG_ON bootloaders wipe keys when uploading unsigned firmwares.
  • Loading branch information
keepkeyjon committed Oct 30, 2018
1 parent ba8d5b2 commit 76b4c5a
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 25 deletions.
32 changes: 7 additions & 25 deletions lib/firmware/storage.c
Original file line number Diff line number Diff line change
Expand Up @@ -411,48 +411,30 @@ void storage_readStorageV1(Storage *storage, const char *ptr, size_t len) {
memzero(&storage->pub.u2froot, sizeof(storage->pub.u2froot));
storage->pub.u2f_counter = 0;

_Static_assert(sizeof(storage->pub.wrapped_storage_key) == 64,
"(un)wrapped key must be 64 bytes");

_Static_assert(sizeof(storage->pub.storage_key_fingerprint) == 32,
"key fingerprint must be 32 bytes");

// Generate a new storage key.
uint8_t storage_key[64];
random_buffer(storage_key, sizeof(storage_key));

// Wrap the storage key using the user's pin.
uint8_t wrapping_key[64];
storage_deriveWrappingKey(storage->pub.has_pin ? storage->sec.pin : "", wrapping_key);
storage_wrapStorageKey(wrapping_key, storage_key, storage->pub.wrapped_storage_key);

// Fingerprint the storage_key.
storage_keyFingerprint(sessionStorageKey,
shadow_config.storage.pub.storage_key_fingerprint);

if (storage->version == 1) {
storage_resetPolicies(storage);
storage_resetCache(&storage->sec.cache);
} else {
storage_readCacheV1(&storage->sec.cache, ptr + 484, 75);
}

// Encrypt storage before throwing away the storage_key, and secrets.
storage_secMigrate(storage, storage_key, /*encrypt=*/true);
_Static_assert(sizeof(storage->pub.wrapped_storage_key) == 64,
"(un)wrapped key must be 64 bytes");

_Static_assert(sizeof(storage->pub.storage_key_fingerprint) == 32,
"key fingerprint must be 32 bytes");

storage_setPin_impl(storage, storage->sec.pin, sessionStorageKey);

if (storage->pub.has_pin) {
memzero(&storage->sec, sizeof(storage->sec));
memzero(sessionStorageKey, sizeof(sessionStorageKey));
sessionPinCached = false;
storage->has_sec = false;
} else {
memcpy(sessionStorageKey, storage_key, sizeof(storage_key));
sessionPinCached = true;
storage->has_sec = true;
}

memzero(storage_key, sizeof(storage_key));
memzero(wrapping_key, sizeof(wrapping_key));
}

void storage_writeStorageV11(char *ptr, size_t len, const Storage *storage) {
Expand Down
4 changes: 4 additions & 0 deletions tools/bootloader/usb_flash.c
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,9 @@ static bool should_restore(void) {
if (SIG_FLAG == 0)
return false;

#ifdef DEBUG_ON
return true;
#else
// Don't restore if the old firmware was unsigned.
//
// This protects users of custom firmware from having their storage sectors
Expand All @@ -225,6 +228,7 @@ static bool should_restore(void) {

// Otherwise both the old and new must be signed by KeepKey in order to restore.
return sig_status == SIG_OK;
#endif
}

/*
Expand Down

0 comments on commit 76b4c5a

Please sign in to comment.