diff --git a/src/keystore.c b/src/keystore.c index 465f41447..1bbb9db84 100644 --- a/src/keystore.c +++ b/src/keystore.c @@ -422,6 +422,23 @@ keystore_error_t keystore_create_and_store_seed( return keystore_encrypt_and_store_seed(seed, host_entropy_size, password); } +// Checks if the retained seed matches the passed seed. +static bool _check_retained_seed(const uint8_t* seed, size_t seed_length) +{ + if (!_is_unlocked_device) { + return false; + } + uint8_t seed_hashed[32] = {0}; + UTIL_CLEANUP_32(seed_hashed); + if (_hash_seed(seed, seed_length, seed_hashed) != KEYSTORE_OK) { + return false; + } + if (!MEMEQ(seed_hashed, _retained_seed_hash, sizeof(_retained_seed_hash))) { + return false; + } + return true; +} + keystore_error_t keystore_unlock( const char* password, uint8_t* remaining_attempts_out, @@ -455,12 +472,7 @@ keystore_error_t keystore_unlock( if (result == KEYSTORE_OK) { if (_is_unlocked_device) { // Already unlocked. Fail if the seed changed under our feet (should never happen). - uint8_t current_seed[KEYSTORE_MAX_SEED_LENGTH] = {0}; - size_t current_seed_len = 0; - if (!keystore_copy_seed(current_seed, ¤t_seed_len)) { - return KEYSTORE_ERR_DECRYPT; - } - if (seed_len != current_seed_len || !MEMEQ(current_seed, seed, current_seed_len)) { + if (!_check_retained_seed(seed, seed_len)) { Abort("Seed has suddenly changed. This should never happen."); } } else { @@ -491,15 +503,9 @@ bool keystore_unlock_bip39_check(const uint8_t* seed, size_t seed_length) return false; } - uint8_t seed_hashed[32] = {0}; - UTIL_CLEANUP_32(seed_hashed); - if (_hash_seed(seed, seed_length, seed_hashed) != KEYSTORE_OK) { + if (!_check_retained_seed(seed, seed_length)) { return false; } - if (!MEMEQ(seed_hashed, _retained_seed_hash, sizeof(_retained_seed_hash))) { - return false; - } - usb_processing_timeout_reset(LONG_TIMEOUT); return true; diff --git a/src/rust/bitbox02-rust/src/hww/api/show_mnemonic.rs b/src/rust/bitbox02-rust/src/hww/api/show_mnemonic.rs index 241561879..b44e11234 100644 --- a/src/rust/bitbox02-rust/src/hww/api/show_mnemonic.rs +++ b/src/rust/bitbox02-rust/src/hww/api/show_mnemonic.rs @@ -149,7 +149,7 @@ mod tests { block_on(process(&mut mock_hal)), Ok(Response::Success(pb::Success {})) ); - assert_eq!(bitbox02::securechip::fake_event_counter(), 7); + assert_eq!(bitbox02::securechip::fake_event_counter(), 6); assert_eq!( mock_hal.ui.screens, diff --git a/src/rust/bitbox02/src/keystore.rs b/src/rust/bitbox02/src/keystore.rs index e4193b172..e0bc5eab2 100644 --- a/src/rust/bitbox02/src/keystore.rs +++ b/src/rust/bitbox02/src/keystore.rs @@ -466,15 +466,18 @@ mod tests { assert!(encrypt_and_store_seed(&seed, "password").is_ok()); lock(); + // First call: unlock. The first one does a seed rentention (1 securechip event). + crate::securechip::fake_event_counter_reset(); + assert!(unlock("password").is_ok()); + assert_eq!(crate::securechip::fake_event_counter(), 6); + // Loop to check that unlocking works while unlocked. - for _ in 0..3 { - // First call: unlock Further calls perform a password check. The first onedoes a seed - // rentention (1 securechip event). The password check does not do the rentention but a - // copy_seed() instead to check the seed, so they end up hacing the same number of - // events.crate::securechip::fake_event_counter_reset(); + for _ in 0..2 { + // Further calls perform a password check.The password check does not do the retention + // so it ends up needing one secure chip operation less. crate::securechip::fake_event_counter_reset(); assert!(unlock("password").is_ok()); - assert_eq!(crate::securechip::fake_event_counter(), 6); + assert_eq!(crate::securechip::fake_event_counter(), 5); } // Also check that the retained seed was encrypted with the expected encryption key.