From 8288bb08600af7012d061e03b8e21257ab2493e6 Mon Sep 17 00:00:00 2001 From: kaczmarczyck <43844792+kaczmarczyck@users.noreply.github.com> Date: Thu, 1 Sep 2022 18:28:03 +0200 Subject: [PATCH] Firmware version for upgrades (#542) * shows and checks the firmware version * merges metadata ranges in boards * simplifies locations loop --- boards/nordic/nrf52840dk_opensk_a/build.rs | 4 +- boards/nordic/nrf52840dk_opensk_b/build.rs | 2 +- src/api/upgrade_storage/mod.rs | 5 +- src/ctap/mod.rs | 3 +- src/env/test/upgrade_storage.rs | 4 ++ src/env/tock/storage.rs | 58 ++++++++++++++++++---- tools/deploy_partition.py | 9 +++- 7 files changed, 69 insertions(+), 16 deletions(-) diff --git a/boards/nordic/nrf52840dk_opensk_a/build.rs b/boards/nordic/nrf52840dk_opensk_a/build.rs index d30899a..4419b7f 100644 --- a/boards/nordic/nrf52840dk_opensk_a/build.rs +++ b/boards/nordic/nrf52840dk_opensk_a/build.rs @@ -25,8 +25,8 @@ static mut STORAGE_LOCATIONS: [kernel::StorageLocation; 5] = [ }, // Partitions can also be split to maximize MPU happiness. kernel::StorageLocation { - address: 0x5000, - size: 0x1000, + address: 0x4000, + size: 0x2000, storage_type: kernel::StorageType::Partition, }, kernel::StorageLocation { diff --git a/boards/nordic/nrf52840dk_opensk_b/build.rs b/boards/nordic/nrf52840dk_opensk_b/build.rs index 400b25b..729440f 100644 --- a/boards/nordic/nrf52840dk_opensk_b/build.rs +++ b/boards/nordic/nrf52840dk_opensk_b/build.rs @@ -26,7 +26,7 @@ static mut STORAGE_LOCATIONS: [kernel::StorageLocation; 5] = [ // Partitions can also be split to maximize MPU happiness. kernel::StorageLocation { address: 0x4000, - size: 0x1000, + size: 0x2000, storage_type: kernel::StorageType::Partition, }, kernel::StorageLocation { diff --git a/src/api/upgrade_storage/mod.rs b/src/api/upgrade_storage/mod.rs index 408fcf1..3d93510 100644 --- a/src/api/upgrade_storage/mod.rs +++ b/src/api/upgrade_storage/mod.rs @@ -1,4 +1,4 @@ -// Copyright 2021 Google LLC +// Copyright 2021-2022 Google LLC // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -53,4 +53,7 @@ pub trait UpgradeStorage { /// /// Returns [`StorageError::OutOfBounds`] if the data is too long to fit the metadata storage. fn write_metadata(&mut self, data: &[u8]) -> StorageResult<()>; + + /// Returns the currently running firmware version. + fn running_firmware_version(&self) -> u64; } diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index c440ef0..2118b5f 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -1295,7 +1295,7 @@ impl CtapState { ), force_pin_change: Some(storage::has_force_pin_change(env)?), min_pin_length: storage::min_pin_length(env)?, - firmware_version: None, + firmware_version: env.upgrade_storage().map(|u| u.running_firmware_version()), max_cred_blob_length: Some(env.customization().max_cred_blob_length() as u64), max_rp_ids_for_set_min_pin_length: Some( env.customization().max_rp_ids_length() as u64 @@ -1590,6 +1590,7 @@ mod test { 0x0B => env.customization().max_large_blob_array_size() as u64, 0x0C => false, 0x0D => storage::min_pin_length(&mut env).unwrap() as u64, + 0x0E => 0, 0x0F => env.customization().max_cred_blob_length() as u64, 0x10 => env.customization().max_rp_ids_length() as u64, 0x14 => storage::remaining_credentials(&mut env).unwrap() as u64, diff --git a/src/env/test/upgrade_storage.rs b/src/env/test/upgrade_storage.rs index 04e3e55..641694f 100644 --- a/src/env/test/upgrade_storage.rs +++ b/src/env/test/upgrade_storage.rs @@ -84,6 +84,10 @@ impl UpgradeStorage for BufferUpgradeStorage { Err(StorageError::OutOfBounds) } } + + fn running_firmware_version(&self) -> u64 { + 0 + } } #[cfg(test)] diff --git a/src/env/tock/storage.rs b/src/env/tock/storage.rs index a8ba7db..1c349b5 100644 --- a/src/env/tock/storage.rs +++ b/src/env/tock/storage.rs @@ -25,6 +25,7 @@ use libtock_core::{callback, syscalls}; use persistent_store::{Storage, StorageError, StorageIndex, StorageResult}; const DRIVER_NUMBER: usize = 0x50003; +const METADATA_SIGN_OFFSET: usize = 0x800; const UPGRADE_PUBLIC_KEY: &[u8; 65] = include_bytes!(concat!(env!("OUT_DIR"), "/opensk_upgrade_pubkey.bin")); @@ -100,6 +101,10 @@ fn block_command(driver: usize, cmd: usize, arg1: usize, arg2: usize) -> Storage } } +unsafe fn read_slice(address: usize, length: usize) -> &'static [u8] { + core::slice::from_raw_parts(address as *const u8, length) +} + fn write_slice(ptr: usize, value: &[u8]) -> StorageResult<()> { let code = unsafe { syscalls::raw::allow( @@ -228,11 +233,15 @@ pub struct TockUpgradeStorage { page_size: usize, partition: ModRange, metadata: ModRange, + running_metadata: ModRange, } impl TockUpgradeStorage { - const METADATA_ADDRESS_A: usize = 0x4000; - const METADATA_ADDRESS_B: usize = 0x5000; + // Ideally, the kernel should tell us metadata and partitions directly. + // This code only works for one layout, refactor this into the storage driver to support more. + const METADATA_ADDRESS: usize = 0x4000; + const _PARTITION_ADDRESS_A: usize = 0x20000; + const PARTITION_ADDRESS_B: usize = 0x60000; /// Provides access to the other upgrade partition and metadata if available. /// @@ -244,7 +253,7 @@ impl TockUpgradeStorage { /// Returns `CustomError` if any of the following conditions do not hold: /// - The page size is a power of two. /// - The storage slices are page-aligned. - /// - There are not partition or metadata slices. + /// - There are no partition or no metadata slices. /// Returns a `NotAligned` error if partitions or metadata ranges are /// - not exclusive or, /// - not consecutive. @@ -253,6 +262,7 @@ impl TockUpgradeStorage { page_size: get_info(command_nr::get_info_nr::PAGE_SIZE, 0)?, partition: ModRange::new_empty(), metadata: ModRange::new_empty(), + running_metadata: ModRange::new_empty(), }; if !locations.page_size.is_power_of_two() { return Err(StorageError::CustomError); @@ -269,7 +279,12 @@ impl TockUpgradeStorage { } let range = ModRange::new(storage_ptr, storage_len); match range.start() { - Self::METADATA_ADDRESS_A | Self::METADATA_ADDRESS_B => locations.metadata = range, + Self::METADATA_ADDRESS => { + // Will be swapped if we are on B. + locations.metadata = ModRange::new(range.start(), locations.page_size); + locations.running_metadata = + ModRange::new(range.start() + locations.page_size, locations.page_size); + } _ => { locations.partition = locations .partition @@ -278,9 +293,15 @@ impl TockUpgradeStorage { } } } - if locations.partition.is_empty() { + if locations.partition.is_empty() + || locations.metadata.is_empty() + || locations.running_metadata.is_empty() + { return Err(StorageError::CustomError); } + if locations.partition.start() == Self::PARTITION_ADDRESS_B { + core::mem::swap(&mut locations.metadata, &mut locations.running_metadata); + } Ok(locations) } @@ -299,7 +320,7 @@ impl UpgradeStorage for TockUpgradeStorage { .partition .contains_range(&ModRange::new(address, length)) { - Ok(unsafe { core::slice::from_raw_parts(address as *const u8, length) }) + Ok(unsafe { read_slice(address, length) }) } else { Err(StorageError::OutOfBounds) } @@ -332,9 +353,7 @@ impl UpgradeStorage for TockUpgradeStorage { } fn read_metadata(&self) -> StorageResult<&[u8]> { - Ok(unsafe { - core::slice::from_raw_parts(self.metadata.start() as *const u8, self.metadata.length()) - }) + Ok(unsafe { read_slice(self.metadata.start(), self.metadata.length()) }) } fn write_metadata(&mut self, data: &[u8]) -> StorageResult<()> { @@ -348,6 +367,16 @@ impl UpgradeStorage for TockUpgradeStorage { } write_slice(self.metadata.start(), data) } + + fn running_firmware_version(&self) -> u64 { + let running_metadata = unsafe { + read_slice( + self.running_metadata.start(), + self.running_metadata.length(), + ) + }; + parse_metadata_version(running_metadata) + } } /// Parses the metadata of an upgrade, and checks its correctness. @@ -367,11 +396,15 @@ fn parse_metadata( metadata: &[u8], ) -> StorageResult<()> { const METADATA_LEN: usize = 0x1000; - const METADATA_SIGN_OFFSET: usize = 0x800; if metadata.len() != METADATA_LEN { return Err(StorageError::CustomError); } + let version = parse_metadata_version(metadata); + if version < upgrade_locations.running_firmware_version() { + return Err(StorageError::CustomError); + } + let metadata_address = LittleEndian::read_u32(&metadata[METADATA_SIGN_OFFSET + 8..][..4]); if metadata_address as usize != upgrade_locations.partition_address() { return Err(StorageError::CustomError); @@ -396,6 +429,11 @@ fn parse_metadata( Ok(()) } +/// Parses the metadata, returns the firmware version. +fn parse_metadata_version(data: &[u8]) -> u64 { + LittleEndian::read_u64(&data[METADATA_SIGN_OFFSET..][..8]) +} + /// Verifies the signature over the given hash. /// /// The public key is COSE encoded, and the hash is a SHA256. diff --git a/tools/deploy_partition.py b/tools/deploy_partition.py index 509cf80..82ad2b3 100755 --- a/tools/deploy_partition.py +++ b/tools/deploy_partition.py @@ -198,6 +198,13 @@ def main(args): aaguid = uuid.UUID(bytes=authenticator.get_info().aaguid) info(f"Upgrading OpenSK device AAGUID {aaguid} ({authenticator.device}).") + running_version = authenticator.get_info().firmware_version + if args.version < running_version: + fatal(f"Can not write version {args.version} when version" + f"{running_version} is running.") + else: + info(f"Running version: {running_version}") + try: check_info(partition_address, authenticator) offset = 0 @@ -225,7 +232,7 @@ def main(args): elif ex.code.value == ctap.CtapError.ERR.INVALID_PARAMETER: error(f"{message} (invalid parameter, maybe a wrong byte array size?).") elif ex.code.value == ctap.CtapError.ERR.INTEGRITY_FAILURE: - error(f"{message} (hashes or signature don't match).") + error(f"{message} (metadata parsing failed).") elif ex.code.value == 0xF2: # VENDOR_INTERNAL_ERROR error(f"{message} (internal conditions not met).") elif ex.code.value == 0xF3: # VENDOR_HARDWARE_FAILURE