What changed, and why it matters
This commit only adds unit tests for the Bluetooth firmware upgrade logic and improves the test-only mock memory implementation so it actually records firmware chunks. There are no changes to production code, no bug fixes, and no security-relevant behavior changes.
No security action needed; this is a test-only change. Standard code review and CI pass are sufficient.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The diff is entirely within test modules and test harness code. It renames an internal helper from _process_upgrade to process_upgrade_helper, adds ble_firmware_slots to TestingMemory, implements realistic chunk writing in ble_firmware_flash_chunk, and adds three unit tests covering chunk boundary requests, metadata/slot updates on success, and correct inactive-slot selection. No production firmware paths are modified.
Changed components
src/rust/bitbox02-rust/src/hal/testing/memory.rssrc/rust/bitbox02-rust/src/hww/api/bluetooth.rsInspect captured patch +196 / −22
diff --git a/src/rust/bitbox02-rust/src/hal/testing/memory.rs b/src/rust/bitbox02-rust/src/hal/testing/memory.rs
index 6e7a624..a3efbc9 100644
--- a/src/rust/bitbox02-rust/src/hal/testing/memory.rs
+++ b/src/rust/bitbox02-rust/src/hal/testing/memory.rs
@@ -10,6 +10,7 @@ use crate::hal::memory::{
pub struct TestingMemory {
ble_enabled: bool,
ble_metadata: BleMetadata,
+ ble_firmware_slots: [Vec<u8>; 2],
active_ble_firmware_version: String,
securechip_type: SecurechipType,
platform: Platform,
@@ -41,6 +42,10 @@ impl TestingMemory {
firmware_sizes: [0; 2],
firmware_checksums: [0; 2],
},
+ ble_firmware_slots: [
+ vec![0xff; crate::hal::memory::BLE_FIRMWARE_MAX_SIZE],
+ vec![0xff; crate::hal::memory::BLE_FIRMWARE_MAX_SIZE],
+ ],
active_ble_firmware_version: "0.0.0".into(),
securechip_type: SecurechipType::Optiga,
platform: Platform::BitBox02,
@@ -90,6 +95,13 @@ impl TestingMemory {
pub fn set_attestation_bootloader_hash(&mut self, hash: &[u8; 32]) {
self.attestation_bootloader_hash = *hash;
}
+
+ pub fn ble_firmware_slot_data(&self, slot: BleFirmwareSlot) -> &[u8] {
+ match slot {
+ BleFirmwareSlot::First => &self.ble_firmware_slots[0],
+ BleFirmwareSlot::Second => &self.ble_firmware_slots[1],
+ }
+ }
}
impl crate::hal::Memory for TestingMemory {
@@ -110,13 +122,29 @@ impl crate::hal::Memory for TestingMemory {
fn ble_firmware_flash_chunk(
&mut self,
- _slot: BleFirmwareSlot,
- _chunk_index: u32,
+ slot: BleFirmwareSlot,
+ chunk_index: u32,
chunk: &[u8],
) -> Result<(), Error> {
if chunk.len() > Self::BLE_FW_FLASH_CHUNK_SIZE as usize {
return Err(Error::InvalidInput);
}
+
+ let chunk_offset = (chunk_index as usize)
+ .checked_mul(Self::BLE_FW_FLASH_CHUNK_SIZE as usize)
+ .ok_or(Error::InvalidInput)?;
+ let chunk_end = chunk_offset
+ .checked_add(chunk.len())
+ .ok_or(Error::InvalidInput)?;
+
+ let slot_data = match slot {
+ BleFirmwareSlot::First => &mut self.ble_firmware_slots[0],
+ BleFirmwareSlot::Second => &mut self.ble_firmware_slots[1],
+ };
+ if chunk_end > slot_data.len() {
+ return Err(Error::InvalidInput);
+ }
+ slot_data[chunk_offset..chunk_end].copy_from_slice(chunk);
Ok(())
}
diff --git a/src/rust/bitbox02-rust/src/hww/api/bluetooth.rs b/src/rust/bitbox02-rust/src/hww/api/bluetooth.rs
index e71faab..4d279de 100644
--- a/src/rust/bitbox02-rust/src/hww/api/bluetooth.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/bluetooth.rs
@@ -50,7 +50,7 @@ trait Funcs {
async fn get_fw_chunk(&mut self, offset: u32, length: u32) -> Result<Vec<u8>, Error>;
}
-async fn _process_upgrade<M: Memory>(
+async fn process_upgrade_helper<M: Memory>(
memory: &mut M,
funcs: &mut impl Funcs,
progress: &mut impl Progress,
@@ -142,7 +142,7 @@ async fn process_upgrade(
.await?;
let mut progress = hal.ui().progress_create("Upgrading...");
- let response = _process_upgrade(
+ let response = process_upgrade_helper(
hal.memory(),
&mut RealFuncs,
&mut progress,
@@ -223,30 +223,60 @@ pub async fn process_api(
mod tests {
use super::*;
- use bitbox02::testing::mock_memory;
+ use crate::hal::memory::{BleFirmwareSlot, BleMetadata};
use util::bb02_async::block_on;
- #[test]
- fn test_chunk_streaming() {
- mock_memory();
+ struct MockFuncs {
+ chunk_requests: Vec<(u32, u32)>,
+ }
- struct MockFuncs {
- chunk_requests: Vec<(u32, u32)>,
+ impl Funcs for MockFuncs {
+ async fn get_fw_chunk(&mut self, offset: u32, length: u32) -> Result<Vec<u8>, Error> {
+ self.chunk_requests.push((offset, length));
+ Ok(vec![0; length as usize])
}
+ }
- impl Funcs for MockFuncs {
- async fn get_fw_chunk(&mut self, offset: u32, length: u32) -> Result<Vec<u8>, Error> {
- self.chunk_requests.push((offset, length));
- Ok(vec![0; length as usize])
- }
+ #[derive(Default)]
+ struct TestProgress {
+ values: Vec<f32>,
+ }
+
+ impl Progress for TestProgress {
+ fn set(&mut self, progress: f32) {
+ self.values.push(progress);
}
+ }
+
+ fn compute_checksum(data: &[u8]) -> u8 {
+ data.iter().fold(0u8, |acc, byte| acc ^ *byte)
+ }
+
+ fn make_metadata(active_index: u8) -> BleMetadata {
+ BleMetadata {
+ allowed_firmware_hash: [9; 32],
+ active_index,
+ firmware_sizes: [11, 22],
+ firmware_checksums: [33, 44],
+ }
+ }
+ fn assert_progress_values(actual: &[f32], expected: &[f32]) {
+ assert_eq!(actual.len(), expected.len());
+ for (actual_value, expected_value) in actual.iter().zip(expected.iter()) {
+ assert!((actual_value - expected_value).abs() < 1e-6);
+ }
+ }
+
+ /// Verifies that successful upgrades request host chunks with exact offset/length boundaries.
+ #[test]
+ fn test_process_upgrade_helper_chunk_streaming() {
struct Test<'a> {
firmware_length: u32,
expected_chunk_requests: &'a [(u32, u32)],
}
- let test_cases = vec![
+ let test_cases = [
Test {
firmware_length: 1,
expected_chunk_requests: &[(0, 1)],
@@ -289,23 +319,139 @@ mod tests {
for test in test_cases {
let mut memory = crate::hal::testing::TestingMemory::new();
let mut mock_funcs = MockFuncs {
- chunk_requests: vec![],
+ chunk_requests: Vec::new(),
};
+ let mut progress = TestProgress::default();
let allowed_hash: [u8; 32] =
Sha256::digest(vec![0; test.firmware_length as usize]).into();
- assert!(
- block_on(_process_upgrade(
+
+ assert_eq!(
+ block_on(process_upgrade_helper(
&mut memory,
&mut mock_funcs,
- &mut crate::hal::testing::ui::NoopProgress,
+ &mut progress,
&pb::BluetoothUpgradeInitRequest {
firmware_length: test.firmware_length,
},
&allowed_hash,
- ))
- .is_ok()
+ )),
+ Ok(Response::Success(pb::BluetoothSuccess {}))
);
assert_eq!(mock_funcs.chunk_requests, test.expected_chunk_requests);
+ assert_eq!(progress.values.len(), test.expected_chunk_requests.len());
+ if !progress.values.is_empty() {
+ let last = *progress.values.last().unwrap();
+ assert!((last - 1.0).abs() < 1e-6);
+ }
}
}
+
+ /// Verifies that a successful upgrade writes firmware bytes to the inactive slot and updates BLE metadata.
+ #[test]
+ fn test_process_upgrade_helper_success_updates_metadata_and_slot_data() {
+ let mut memory = crate::hal::testing::TestingMemory::new();
+ let initial_metadata = make_metadata(0);
+ memory.set_ble_metadata(&initial_metadata).unwrap();
+
+ let firmware = vec![0; 4096 + 5];
+ let allowed_hash: [u8; 32] = Sha256::digest(firmware.as_slice()).into();
+
+ let mut mock_funcs = MockFuncs {
+ chunk_requests: Vec::new(),
+ };
+ let mut progress = TestProgress::default();
+
+ assert_eq!(
+ block_on(process_upgrade_helper(
+ &mut memory,
+ &mut mock_funcs,
+ &mut progress,
+ &pb::BluetoothUpgradeInitRequest {
+ firmware_length: firmware.len() as u32,
+ },
+ &allowed_hash,
+ )),
+ Ok(Response::Success(pb::BluetoothSuccess {}))
+ );
+ assert_eq!(mock_funcs.chunk_requests, vec![(0, 4096), (4096, 5)]);
+ assert_progress_values(&progress.values, &[0.5, 1.0]);
+
+ let metadata = memory.ble_get_metadata();
+ assert_eq!(metadata.active_index, 1);
+ assert_eq!(metadata.allowed_firmware_hash, allowed_hash);
+ assert_eq!(
+ metadata.firmware_sizes[0],
+ initial_metadata.firmware_sizes[0]
+ );
+ assert_eq!(metadata.firmware_sizes[1], firmware.len() as u16);
+ assert_eq!(
+ metadata.firmware_checksums[0],
+ initial_metadata.firmware_checksums[0]
+ );
+ assert_eq!(metadata.firmware_checksums[1], compute_checksum(&firmware));
+
+ assert_eq!(
+ &memory.ble_firmware_slot_data(BleFirmwareSlot::Second)[..firmware.len()],
+ firmware.as_slice()
+ );
+ assert!(
+ memory.ble_firmware_slot_data(BleFirmwareSlot::First)[..firmware.len()]
+ .iter()
+ .all(|&byte| byte == 0xff)
+ );
+ }
+
+ /// Verifies that when slot 1 is active, a successful upgrade targets slot 0 and flips active index.
+ #[test]
+ fn test_process_upgrade_helper_success_uses_first_slot_if_second_is_active() {
+ let mut memory = crate::hal::testing::TestingMemory::new();
+ let initial_metadata = make_metadata(1);
+ memory.set_ble_metadata(&initial_metadata).unwrap();
+
+ let firmware = vec![0; 3];
+ let allowed_hash: [u8; 32] = Sha256::digest(firmware.as_slice()).into();
+ let mut mock_funcs = MockFuncs {
+ chunk_requests: Vec::new(),
+ };
+ let mut progress = TestProgress::default();
+
+ assert_eq!(
+ block_on(process_upgrade_helper(
+ &mut memory,
+ &mut mock_funcs,
+ &mut progress,
+ &pb::BluetoothUpgradeInitRequest {
+ firmware_length: firmware.len() as u32,
+ },
+ &allowed_hash,
+ )),
+ Ok(Response::Success(pb::BluetoothSuccess {}))
+ );
+ assert_eq!(mock_funcs.chunk_requests, vec![(0, 3)]);
+ assert_progress_values(&progress.values, &[1.0]);
+
+ let metadata = memory.ble_get_metadata();
+ assert_eq!(metadata.active_index, 0);
+ assert_eq!(metadata.allowed_firmware_hash, allowed_hash);
+ assert_eq!(metadata.firmware_sizes[0], firmware.len() as u16);
+ assert_eq!(
+ metadata.firmware_sizes[1],
+ initial_metadata.firmware_sizes[1]
+ );
+ assert_eq!(metadata.firmware_checksums[0], compute_checksum(&firmware));
+ assert_eq!(
+ metadata.firmware_checksums[1],
+ initial_metadata.firmware_checksums[1]
+ );
+
+ assert_eq!(
+ &memory.ble_firmware_slot_data(BleFirmwareSlot::First)[..firmware.len()],
+ firmware.as_slice()
+ );
+ assert!(
+ memory.ble_firmware_slot_data(BleFirmwareSlot::Second)[..firmware.len()]
+ .iter()
+ .all(|&byte| byte == 0xff)
+ );
+ }
}
Why this scored 15/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.