workflow/mnemonic: use HAL to get random number
What changed, and why it matters
This commit refactors how random numbers are generated during the backup verification workflow. Previously, the code called a low-level device function directly; now it uses a hardware abstraction layer (HAL) interface so the same code can be tested with a mock random source. The change itself is a code-quality/testability improvement and does not appear to fix an active security vulnerability, but it touches the code that creates the random word-order challenges used to verify a user's seed backup.
No immediate action required. Treat as a normal refactoring commit. Reviewers may want to confirm that the production `Random` HAL implementation still maps to the same secure hardware random source and that no other code paths bypass the HAL for mnemonic-related randomness.
Security signals we found
Refactors randomness source for mnemonic confirmation challenges
Removes direct call to `bitbox02::random::mcu_32_bytes()` from workflow code
Adds mockable `Random` HAL dependency to enable deterministic unit testing
Adds unit test asserting uniqueness and correct placement of target word in random challenge
Evidence from the diff
The patch changes workflow/mnemonic.rs so that create_random_unique_words() and show_and_confirm_mnemonic() accept a &mut impl crate::hal::Random parameter instead of calling bitbox02::random::mcu_32_bytes() directly. The UI HAL trait and its testing mock are updated to pass the random source through. Call sites in bip85.rs and show_mnemonic.rs now split the HAL subsystems to provide both the UI and random interfaces. A new deterministic unit test is added to verify that the random-word selection produces the expected order and uniqueness.
Changed components
src/rust/bitbox02-rust/src/workflow/mnemonic.rssrc/rust/bitbox02-rust/src/hal/ui.rssrc/rust/bitbox02-rust/src/hal/testing/ui.rssrc/rust/bitbox02-rust/src/hww/api/bip85.rssrc/rust/bitbox02-rust/src/hww/api/show_mnemonic.rssrc/rust/bitbox02-rust/src/workflow/mnemonic_c_unit_tests.rsInspect captured patch +59 / −12
diff --git a/src/rust/bitbox02-rust/src/hal/testing/ui.rs b/src/rust/bitbox02-rust/src/hal/testing/ui.rs
index 424aa1a..5ad6297 100644
--- a/src/rust/bitbox02-rust/src/hal/testing/ui.rs
+++ b/src/rust/bitbox02-rust/src/hal/testing/ui.rs
@@ -162,7 +162,11 @@ impl Ui for TestingUi<'_> {
todo!("not used in unit tests yet");
}
- async fn show_and_confirm_mnemonic(&mut self, words: &[&str]) -> Result<(), cancel::Error> {
+ async fn show_and_confirm_mnemonic(
+ &mut self,
+ _random: &mut impl crate::hal::Random,
+ words: &[&str],
+ ) -> Result<(), cancel::Error> {
self.screens.push(Screen::ShowAndConfirmMnemonic {
mnemonic: words.join(" "),
});
diff --git a/src/rust/bitbox02-rust/src/hal/ui.rs b/src/rust/bitbox02-rust/src/hal/ui.rs
index 5f26e25..ce51ed4 100644
--- a/src/rust/bitbox02-rust/src/hal/ui.rs
+++ b/src/rust/bitbox02-rust/src/hal/ui.rs
@@ -62,11 +62,15 @@ pub trait Ui {
///
/// This function is defined in the HAL so unit tests can easily mock it. Real implementations
/// should leave the default implementation.
- async fn show_and_confirm_mnemonic(&mut self, words: &[&str]) -> Result<(), cancel::Error>
+ async fn show_and_confirm_mnemonic(
+ &mut self,
+ random: &mut impl crate::hal::Random,
+ words: &[&str],
+ ) -> Result<(), cancel::Error>
where
Self: Sized,
{
- mnemonic::show_and_confirm_mnemonic(self, words).await
+ mnemonic::show_and_confirm_mnemonic(self, random, words).await
}
/// Retrieve a BIP39 mnemonic sentence of 12 or 24 words from the user.
diff --git a/src/rust/bitbox02-rust/src/hww/api/bip85.rs b/src/rust/bitbox02-rust/src/hww/api/bip85.rs
index 0286bb7..ab77f90 100644
--- a/src/rust/bitbox02-rust/src/hww/api/bip85.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/bip85.rs
@@ -111,7 +111,10 @@ async fn process_bip39(hal: &mut impl crate::hal::Hal) -> Result<(), Error> {
let mnemonic = keystore::bip85_bip39(hal, num_words, index)?;
let words: Vec<&str> = mnemonic.split(' ').collect();
- hal.ui().show_and_confirm_mnemonic(&words).await?;
+ {
+ let crate::hal::HalSubsystems { ui, random, .. } = hal.subsystems();
+ ui.show_and_confirm_mnemonic(random, &words).await?;
+ }
hal.ui().status("Finished", true).await;
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 1ee7649..d2c739e 100644
--- a/src/rust/bitbox02-rust/src/hww/api/show_mnemonic.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/show_mnemonic.rs
@@ -45,7 +45,10 @@ pub async fn process(hal: &mut impl crate::hal::Hal) -> Result<Response, Error>
let words: Vec<&str> = mnemonic_sentence.split(' ').collect();
- hal.ui().show_and_confirm_mnemonic(&words).await?;
+ {
+ let crate::hal::HalSubsystems { ui, random, .. } = hal.subsystems();
+ ui.show_and_confirm_mnemonic(random, &words).await?;
+ }
hal.memory().set_initialized().or(Err(Error::Memory))?;
diff --git a/src/rust/bitbox02-rust/src/workflow/mnemonic.rs b/src/rust/bitbox02-rust/src/workflow/mnemonic.rs
index 3615891..5eeea06 100644
--- a/src/rust/bitbox02-rust/src/workflow/mnemonic.rs
+++ b/src/rust/bitbox02-rust/src/workflow/mnemonic.rs
@@ -24,14 +24,18 @@ fn as_str_vec(v: &[zeroize::Zeroizing<String>]) -> Vec<&str> {
/// one of them is provided `word`. Returns the position of `word` in
/// the list of words, and the lis of words. This is used to test if
/// the user wrote down the seed words properly.
-fn create_random_unique_words(word: &str, length: u8) -> (u8, Vec<zeroize::Zeroizing<String>>) {
- fn rand16() -> u16 {
+fn create_random_unique_words(
+ hal_random: &mut impl crate::hal::Random,
+ word: &str,
+ length: u8,
+) -> (u8, Vec<zeroize::Zeroizing<String>>) {
+ fn rand16(hal_random: &mut impl crate::hal::Random) -> u16 {
let mut rand = [0u8; 32];
- bitbox02::random::mcu_32_bytes(&mut rand);
+ hal_random.mcu_32_bytes(&mut rand);
((rand[0] as u16) << 8) | (rand[1] as u16)
}
- let index_word = (rand16() as u8) % length;
+ let index_word = (rand16(hal_random) as u8) % length;
let mut picked_indices = Vec::new();
let result = (0..length)
.map(|i| {
@@ -43,7 +47,7 @@ fn create_random_unique_words(word: &str, length: u8) -> (u8, Vec<zeroize::Zeroi
// A random word everywhere else.
// Loop until we get a unique word, we don't want repeated words in the list.
loop {
- let idx = rand16() % BIP39_WORDLIST_LEN;
+ let idx = rand16(hal_random) % BIP39_WORDLIST_LEN;
if picked_indices.contains(&idx) {
continue;
};
@@ -96,6 +100,7 @@ pub async fn confirm_word(choices: &[&str], title: &str) -> Result<u8, CancelErr
pub async fn show_and_confirm_mnemonic(
hal_ui: &mut impl crate::hal::Ui,
+ hal_random: &mut impl crate::hal::Random,
words: &[&str],
) -> Result<(), CancelError> {
hal_ui
@@ -125,7 +130,7 @@ pub async fn show_and_confirm_mnemonic(
// Part 2) Confirm words
for (word_idx, word) in words.iter().enumerate() {
let title = format!("{:02}", word_idx + 1);
- let (correct_idx, choices) = create_random_unique_words(word, NUM_RANDOM_WORDS);
+ let (correct_idx, choices) = create_random_unique_words(hal_random, word, NUM_RANDOM_WORDS);
let mut choices: Vec<&str> = choices.iter().map(|c| c.as_ref()).collect();
choices.push("Back to\nrecovery words");
let back_idx = (choices.len() - 1) as u8;
@@ -415,7 +420,12 @@ pub async fn get(
mod tests {
use super::*;
- use alloc::boxed::Box;
+ fn u16_to_rand(value: u16) -> [u8; 32] {
+ let mut out = [0u8; 32];
+ out[0] = (value >> 8) as u8;
+ out[1] = value as u8;
+ out
+ }
fn bruteforce_lastword(mnemonic: &[&str]) -> Vec<zeroize::Zeroizing<String>> {
let mut result = Vec::new();
@@ -430,6 +440,28 @@ mod tests {
result
}
+ #[test]
+ fn test_create_random_unique_words() {
+ let mut random = crate::hal::testing::TestingRandom::new();
+ random.mock_next(u16_to_rand(2)); // place the target at index 2 in a 5-entry list.
+ random.mock_next(u16_to_rand(0));
+ random.mock_next(u16_to_rand(1));
+ random.mock_next(u16_to_rand(2));
+ random.mock_next(u16_to_rand(3));
+ let (correct_idx, choices) =
+ create_random_unique_words(&mut random, "zoo", NUM_RANDOM_WORDS);
+ assert_eq!(correct_idx, 2);
+ assert_eq!(
+ as_str_vec(&choices),
+ vec!["abandon", "ability", "zoo", "able", "about"]
+ );
+
+ let mut unique = as_str_vec(&choices);
+ unique.sort_unstable();
+ unique.dedup();
+ assert_eq!(unique.len(), choices.len());
+ }
+
#[test]
fn test_lastword_choices() {
// 23 words
diff --git a/src/rust/bitbox02-rust/src/workflow/mnemonic_c_unit_tests.rs b/src/rust/bitbox02-rust/src/workflow/mnemonic_c_unit_tests.rs
index fc1bb4d..a87820a 100644
--- a/src/rust/bitbox02-rust/src/workflow/mnemonic_c_unit_tests.rs
+++ b/src/rust/bitbox02-rust/src/workflow/mnemonic_c_unit_tests.rs
@@ -15,6 +15,7 @@ pub async fn confirm_word(_choices: &[&str], _title: &str) -> Result<u8, CancelE
pub async fn show_and_confirm_mnemonic(
_ui: &mut impl crate::hal::Ui,
+ _random: &mut impl crate::hal::Random,
words: &[&str],
) -> Result<(), CancelError> {
for word in words.iter() {
Why this scored 29/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.