diff --git a/src/api/upgrade_storage/helper.rs b/src/api/upgrade_storage/helper.rs index a2a6feb..cac62d4 100644 --- a/src/api/upgrade_storage/helper.rs +++ b/src/api/upgrade_storage/helper.rs @@ -15,6 +15,7 @@ // For compiling with std outside of tests. #![cfg_attr(feature = "std", allow(dead_code))] +use alloc::vec::Vec; use core::iter::Iterator; use persistent_store::{StorageError, StorageResult}; @@ -55,6 +56,7 @@ pub fn is_aligned(block_size: usize, address: usize) -> bool { /// /// The range is treated as the interval `[start, start + length)`. /// All objects with length of 0, regardless of the start value, are considered empty. +#[derive(Clone, Debug, PartialEq, Eq)] pub struct ModRange { start: usize, length: usize, @@ -86,25 +88,45 @@ impl ModRange { self.length == 0 } - /// Returns the disjoint union with the other range, if is consecutive. + /// Returns the disjoint union with the other range, if consecutive. /// /// Appending empty ranges is not possible. /// Appending to the empty range returns the other range. - pub fn append(&self, other: ModRange) -> Option { + /// + /// Returns true if successful. + pub fn append(&mut self, other: &ModRange) -> bool { if self.is_empty() { - return Some(other); + self.start = other.start; + self.length = other.length; + return true; } if other.is_empty() { - return None; + return false; } if self.start >= other.start { - return None; + return false; } if self.length != other.start - self.start { - return None; + return false; } - let new_length = self.length.checked_add(other.length); - new_length.map(|l| ModRange::new(self.start, l)) + if let Some(new_length) = self.length.checked_add(other.length) { + self.length = new_length; + true + } else { + false + } + } + + /// Helper function to check whether a range starts within another. + fn starts_inside(&self, range: &ModRange) -> bool { + !range.is_empty() && self.start >= range.start && self.start - range.start < range.length + } + + /// Returns whether the given range has intersects. + /// + /// Mathematically, we calculate whether: `self ∩ range ≠ ∅`. + pub fn intersects_range(&self, range: &ModRange) -> bool { + self.starts_inside(range) || range.starts_inside(self) } /// Returns whether the given range is fully contained. @@ -128,6 +150,73 @@ impl ModRange { } } +pub struct Partition { + ranges: Vec, +} + +impl Partition { + pub fn new() -> Partition { + Partition { ranges: Vec::new() } + } + + /// Total length of all ranges. + pub fn length(&self) -> usize { + self.ranges.iter().map(|r| r.length()).sum() + } + + /// Appends the given range. + /// + /// Ranges should be appending with ascending start addresses. + pub fn append(&mut self, range: ModRange) -> bool { + if let Some(last_range) = self.ranges.last_mut() { + if range.start() <= last_range.start() + || range.start() - last_range.start() < last_range.length() + { + return false; + } + if !last_range.append(&range) { + self.ranges.push(range); + } + } else { + self.ranges.push(range); + } + true + } + + /// Returns the start address that corresponds to the given offset. + /// + /// If the offset bigger than the accumulated length or the requested slice doesn't fit a + /// connected component, return `None`. + pub fn find_address(&self, mut offset: usize, length: usize) -> Option { + for range in &self.ranges { + if offset < range.length() { + return if range.length() - offset >= length { + Some(range.start() + offset) + } else { + None + }; + } + offset -= range.length() + } + None + } + + pub fn ranges_from(&self, start_address: usize) -> Vec { + let mut result = Vec::new(); + for range in &self.ranges { + match start_address.checked_sub(range.start()) { + None | Some(0) => result.push(range.clone()), + Some(offset) => { + if range.length() > offset { + result.push(ModRange::new(start_address, range.length() - offset)); + } + } + } + } + result + } +} + #[cfg(test)] mod tests { use super::*; @@ -186,18 +275,17 @@ mod tests { #[test] fn mod_range_append() { - let range = ModRange::new(200, 100); - let new_range = range.append(ModRange::new(300, 400)).unwrap(); - assert!(new_range.start() == 200); - assert!(new_range.length() == 500); - assert!(range.append(ModRange::new(299, 400)).is_none()); - assert!(range.append(ModRange::new(301, 400)).is_none()); - assert!(range.append(ModRange::new(200, 400)).is_none()); - let empty_append = ModRange::new_empty() - .append(ModRange::new(200, 100)) - .unwrap(); - assert!(empty_append.start() == 200); - assert!(empty_append.length() == 100); + let mut range = ModRange::new(200, 100); + assert!(range.append(&ModRange::new(300, 400))); + assert!(range.start() == 200); + assert!(range.length() == 500); + assert!(!range.append(&ModRange::new(499, 400))); + assert!(!range.append(&ModRange::new(501, 400))); + assert!(!range.append(&ModRange::new(300, 400))); + let mut range = ModRange::new_empty(); + assert!(range.append(&ModRange::new(200, 100))); + assert!(range.start() == 200); + assert!(range.length() == 100); } #[test] @@ -216,6 +304,20 @@ mod tests { assert!(ModRange::new(usize::MAX, 2).contains_range(&ModRange::new(usize::MAX, 2))); } + #[test] + fn mod_range_intersects_range() { + let range = ModRange::new(200, 100); + assert!(range.intersects_range(&ModRange::new(200, 1))); + assert!(range.intersects_range(&ModRange::new(299, 1))); + assert!(!range.intersects_range(&ModRange::new(199, 1))); + assert!(!range.intersects_range(&ModRange::new(300, 1))); + assert!(!ModRange::new_empty().intersects_range(&ModRange::new_empty())); + assert!(!ModRange::new_empty().intersects_range(&ModRange::new(200, 100))); + assert!(!ModRange::new(200, 100).intersects_range(&ModRange::new_empty())); + assert!(ModRange::new(usize::MAX, 1).intersects_range(&ModRange::new(usize::MAX, 1))); + assert!(ModRange::new(usize::MAX, 2).intersects_range(&ModRange::new(usize::MAX, 2))); + } + #[test] fn mod_range_aligned_iter() { let mut iter = ModRange::new(200, 100).aligned_iter(100); @@ -234,4 +336,49 @@ mod tests { assert_eq!(iter.next(), Some(0xffff_ffff_ffff_fff0)); assert_eq!(iter.next(), None); } + + #[test] + fn partition_append() { + let mut partition = Partition::new(); + partition.append(ModRange::new(0x4000, 0x1000)); + partition.append(ModRange::new(0x20000, 0x20000)); + partition.append(ModRange::new(0x40000, 0x20000)); + assert_eq!(partition.find_address(0, 1), Some(0x4000)); + assert_eq!(partition.length(), 0x41000); + } + + #[test] + fn partition_find_address() { + let mut partition = Partition::new(); + partition.append(ModRange::new(0x4000, 0x1000)); + partition.append(ModRange::new(0x20000, 0x20000)); + partition.append(ModRange::new(0x40000, 0x20000)); + assert_eq!(partition.find_address(0, 0x1000), Some(0x4000)); + assert_eq!(partition.find_address(0x1000, 0x1000), Some(0x20000)); + assert_eq!(partition.find_address(0x20000, 0x1000), Some(0x3F000)); + assert_eq!(partition.find_address(0x21000, 0x1000), Some(0x40000)); + assert_eq!(partition.find_address(0x40000, 0x1000), Some(0x5F000)); + assert_eq!(partition.find_address(0x41000, 0x1000), None); + assert_eq!(partition.find_address(0x40000, 0x2000), None); + } + + #[test] + fn partition_ranges_from() { + let mut partition = Partition::new(); + partition.append(ModRange::new(0x4000, 0x1000)); + partition.append(ModRange::new(0x20000, 0x20000)); + partition.append(ModRange::new(0x40000, 0x20000)); + let all_ranges = partition.ranges_from(0); + let from_start_ranges = partition.ranges_from(0x4000); + assert_eq!(&all_ranges, &from_start_ranges); + assert_eq!(all_ranges.len(), 2); + assert_eq!(all_ranges[0], ModRange::new(0x4000, 0x1000)); + assert_eq!(all_ranges[1], ModRange::new(0x20000, 0x40000)); + let second_range = partition.ranges_from(0x20000); + let same_second_range = partition.ranges_from(0x1F000); + assert_eq!(&second_range, &same_second_range); + assert_eq!(&second_range, &all_ranges[1..]); + let partial_range = partition.ranges_from(0x30000); + assert_eq!(partial_range[0], ModRange::new(0x30000, 0x30000)); + } } diff --git a/src/api/upgrade_storage/mod.rs b/src/api/upgrade_storage/mod.rs index 3d93510..9426165 100644 --- a/src/api/upgrade_storage/mod.rs +++ b/src/api/upgrade_storage/mod.rs @@ -20,7 +20,9 @@ pub(crate) mod helper; pub trait UpgradeStorage { /// Reads a slice of the partition, if within bounds. /// - /// The offset is relative to the start of the partition. + /// The offset is relative to the start of the partition, excluding holes. The partition is + /// presented as one connected component. Therefore, the offset does not easily translate + /// to physical memory address address of the slice. /// /// # Errors /// @@ -29,31 +31,23 @@ pub trait UpgradeStorage { /// Writes the given data to the given offset address, if within bounds of the partition. /// - /// The offset is relative to the start of the partition. + /// The offset is relative to the start of the partition, excluding holes. + /// See `read_partition`. /// /// # Errors /// - /// Returns [`StorageError::OutOfBounds`] if the data does not fit the partition. + /// - Returns [`StorageError::OutOfBounds`] if the data does not fit the partition. + /// - Returns [`StorageError::CustomError`] if any Metadata check fails. fn write_partition(&mut self, offset: usize, data: &[u8]) -> StorageResult<()>; - /// Returns the address of the partition. - fn partition_address(&self) -> usize; + /// Returns an identifier for the partition. + /// + /// Use this to determine whether you are writing to A or B. + fn partition_identifier(&self) -> u32; /// Returns the length of the partition. fn partition_length(&self) -> usize; - /// Reads the metadata location. - fn read_metadata(&self) -> StorageResult<&[u8]>; - - /// Writes the given data into the metadata location. - /// - /// The passed in data is appended with 0xFF bytes if shorter than the metadata storage. - /// - /// # Errors - /// - /// 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/command.rs b/src/ctap/command.rs index 2464eb5..cd98989 100644 --- a/src/ctap/command.rs +++ b/src/ctap/command.rs @@ -592,7 +592,7 @@ impl TryFrom for AuthenticatorVendorConfigureParameters { #[derive(Debug, PartialEq, Eq)] pub struct AuthenticatorVendorUpgradeParameters { - pub address: Option, + pub offset: usize, pub data: Vec, pub hash: Vec, } @@ -603,22 +603,15 @@ impl TryFrom for AuthenticatorVendorUpgradeParameters { fn try_from(cbor_value: cbor::Value) -> Result { destructure_cbor_map! { let { - 0x01 => address, + 0x01 => offset, 0x02 => data, 0x03 => hash, } = extract_map(cbor_value)?; } - let address = address - .map(extract_unsigned) - .transpose()? - .map(|u| u as usize); + let offset = extract_unsigned(ok_or_missing(offset)?)? as usize; let data = extract_byte_string(ok_or_missing(data)?)?; let hash = extract_byte_string(ok_or_missing(hash)?)?; - Ok(AuthenticatorVendorUpgradeParameters { - address, - data, - hash, - }) + Ok(AuthenticatorVendorUpgradeParameters { offset, data, hash }) } } @@ -1067,6 +1060,16 @@ mod test { let command = Command::deserialize(&cbor_bytes); assert_eq!(command, Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR)); + // Missing offset + let cbor_value = cbor_map! { + 0x02 => [0xFF; 0x100], + 0x03 => [0x44; 32], + }; + assert_eq!( + AuthenticatorVendorUpgradeParameters::try_from(cbor_value), + Err(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER) + ); + // Missing data let cbor_value = cbor_map! { 0x01 => 0x1000, @@ -1087,21 +1090,7 @@ mod test { Err(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER) ); - // Valid without address - let cbor_value = cbor_map! { - 0x02 => [0xFF; 0x100], - 0x03 => [0x44; 32], - }; - assert_eq!( - AuthenticatorVendorUpgradeParameters::try_from(cbor_value), - Ok(AuthenticatorVendorUpgradeParameters { - address: None, - data: vec![0xFF; 0x100], - hash: vec![0x44; 32], - }) - ); - - // Valid with address + // Valid let cbor_value = cbor_map! { 0x01 => 0x1000, 0x02 => [0xFF; 0x100], @@ -1110,7 +1099,7 @@ mod test { assert_eq!( AuthenticatorVendorUpgradeParameters::try_from(cbor_value), Ok(AuthenticatorVendorUpgradeParameters { - address: Some(0x1000), + offset: 0x1000, data: vec![0xFF; 0x100], hash: vec![0x44; 32], }) diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 2118b5f..6ca77f5 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -1398,30 +1398,16 @@ impl CtapState { env: &mut impl Env, params: AuthenticatorVendorUpgradeParameters, ) -> Result { - let AuthenticatorVendorUpgradeParameters { - address, - data, - hash, - } = params; + let AuthenticatorVendorUpgradeParameters { offset, data, hash } = params; let upgrade_locations = env .upgrade_storage() .ok_or(Ctap2StatusCode::CTAP1_ERR_INVALID_COMMAND)?; - let written_slice = if let Some(address) = address { - upgrade_locations - .write_partition(address, &data) - .map_err(|_| Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)?; - upgrade_locations - .read_partition(address, data.len()) - .map_err(|_| Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)? - } else { - // Write the metadata page after verifying it. - upgrade_locations - .write_metadata(&data) - .map_err(|_| Ctap2StatusCode::CTAP2_ERR_INTEGRITY_FAILURE)?; - &upgrade_locations - .read_metadata() - .map_err(|_| Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)?[..data.len()] - }; + upgrade_locations + .write_partition(offset, &data) + .map_err(|_| Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)?; + let written_slice = upgrade_locations + .read_partition(offset, data.len()) + .map_err(|_| Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)?; let written_hash = Sha256::hash(written_slice); if hash != written_hash { return Err(Ctap2StatusCode::CTAP2_ERR_INTEGRITY_FAILURE); @@ -1438,7 +1424,7 @@ impl CtapState { .ok_or(Ctap2StatusCode::CTAP1_ERR_INVALID_COMMAND)?; Ok(ResponseData::AuthenticatorVendorUpgradeInfo( AuthenticatorVendorUpgradeInfoResponse { - info: upgrade_locations.partition_address() as u32, + info: upgrade_locations.partition_identifier(), }, )) } @@ -1493,7 +1479,6 @@ mod test { use crate::api::user_presence::UserPresenceResult; use crate::env::test::TestEnv; use crate::test_helpers; - use byteorder::LittleEndian; use cbor::{cbor_array, cbor_array_vec, cbor_map}; // The keep-alive logic in the processing of some commands needs a channel ID to send @@ -3404,70 +3389,63 @@ mod test { // The test metadata storage has size 0x1000. // The test identifier matches partition B. let mut env = TestEnv::new(); - let private_key = ecdsa::SecKey::gensk(env.rng()); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); - const METADATA_LEN: usize = 0x1000; - const METADATA_SIGN_OFFSET: usize = 0x800; - let mut metadata = vec![0xFF; METADATA_LEN]; - LittleEndian::write_u32(&mut metadata[METADATA_SIGN_OFFSET + 8..][..4], 0x60000); + const METADATA_LEN: usize = 0x1000; + let metadata = vec![0xFF; METADATA_LEN]; + let metadata_hash = Sha256::hash(&metadata).to_vec(); let data = vec![0xFF; 0x1000]; let hash = Sha256::hash(&data).to_vec(); - let upgrade_locations = env.upgrade_storage().unwrap(); - let partition_length = upgrade_locations.partition_length(); - let mut signed_over_data = metadata[METADATA_SIGN_OFFSET..].to_vec(); - signed_over_data.extend( - upgrade_locations - .read_partition(0, partition_length) - .unwrap(), - ); - let signed_hash = Sha256::hash(&signed_over_data); - - metadata[..32].copy_from_slice(&signed_hash); - let signature = private_key.sign_rfc6979::(&signed_over_data); - let mut signature_bytes = [0; ecdsa::Signature::BYTES_LENGTH]; - signature.to_bytes(&mut signature_bytes); - metadata[32..96].copy_from_slice(&signature_bytes); - let metadata_hash = Sha256::hash(&metadata).to_vec(); // Write to partition. let response = ctap_state.process_vendor_upgrade( &mut env, AuthenticatorVendorUpgradeParameters { - address: Some(0x20000), + offset: 0x20000, data: data.clone(), hash: hash.clone(), }, ); assert_eq!(response, Ok(ResponseData::AuthenticatorVendorUpgrade)); - // TestEnv doesn't check the metadata, test parsing that in your Env. + // TestEnv doesn't check the metadata, test its parser in your Env. let response = ctap_state.process_vendor_upgrade( &mut env, AuthenticatorVendorUpgradeParameters { - address: None, + offset: 0, data: metadata.clone(), hash: metadata_hash.clone(), }, ); assert_eq!(response, Ok(ResponseData::AuthenticatorVendorUpgrade)); + // TestEnv doesn't check the metadata, test its parser in your Env. + let response = ctap_state.process_vendor_upgrade( + &mut env, + AuthenticatorVendorUpgradeParameters { + offset: METADATA_LEN, + data: data.clone(), + hash: hash.clone(), + }, + ); + assert_eq!(response, Ok(ResponseData::AuthenticatorVendorUpgrade)); + // Write metadata of a wrong size. let response = ctap_state.process_vendor_upgrade( &mut env, AuthenticatorVendorUpgradeParameters { - address: None, + offset: 0, data: metadata[..METADATA_LEN - 1].to_vec(), hash: metadata_hash, }, ); - assert_eq!(response, Err(Ctap2StatusCode::CTAP2_ERR_INTEGRITY_FAILURE)); + assert_eq!(response, Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)); // Write outside of the partition. let response = ctap_state.process_vendor_upgrade( &mut env, AuthenticatorVendorUpgradeParameters { - address: Some(0x40000), + offset: 0x41000, data: data.clone(), hash, }, @@ -3478,7 +3456,7 @@ mod test { let response = ctap_state.process_vendor_upgrade( &mut env, AuthenticatorVendorUpgradeParameters { - address: Some(0x20000), + offset: 0x20000, data, hash: [0xEE; 32].to_vec(), }, @@ -3497,7 +3475,7 @@ mod test { let response = ctap_state.process_vendor_upgrade( &mut env, AuthenticatorVendorUpgradeParameters { - address: Some(0), + offset: 0, data, hash, }, @@ -3509,14 +3487,14 @@ mod test { fn test_vendor_upgrade_info() { let mut env = TestEnv::new(); let ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); - let partition_address = env.upgrade_storage().unwrap().partition_address(); + let partition_identifier = env.upgrade_storage().unwrap().partition_identifier(); let upgrade_info_reponse = ctap_state.process_vendor_upgrade_info(&mut env); assert_eq!( upgrade_info_reponse, Ok(ResponseData::AuthenticatorVendorUpgradeInfo( AuthenticatorVendorUpgradeInfoResponse { - info: partition_address as u32, + info: partition_identifier, } )) ); diff --git a/src/env/test/upgrade_storage.rs b/src/env/test/upgrade_storage.rs index 641694f..5021834 100644 --- a/src/env/test/upgrade_storage.rs +++ b/src/env/test/upgrade_storage.rs @@ -17,22 +17,18 @@ use crate::api::upgrade_storage::UpgradeStorage; use alloc::boxed::Box; use persistent_store::{StorageError, StorageResult}; -const PARTITION_LENGTH: usize = 0x40000; +const PARTITION_LENGTH: usize = 0x41000; const METADATA_LENGTH: usize = 0x1000; pub struct BufferUpgradeStorage { /// Content of the partition storage. partition: Box<[u8]>, - - /// Content of the metadata storage. - metadata: Box<[u8]>, } impl BufferUpgradeStorage { pub fn new() -> StorageResult { Ok(BufferUpgradeStorage { partition: vec![0xff; PARTITION_LENGTH].into_boxed_slice(), - metadata: vec![0xff; METADATA_LENGTH].into_boxed_slice(), }) } } @@ -51,6 +47,9 @@ impl UpgradeStorage for BufferUpgradeStorage { } fn write_partition(&mut self, offset: usize, data: &[u8]) -> StorageResult<()> { + if offset == 0 && data.len() != METADATA_LENGTH { + return Err(StorageError::OutOfBounds); + } if data.is_empty() { return Err(StorageError::OutOfBounds); } @@ -63,7 +62,7 @@ impl UpgradeStorage for BufferUpgradeStorage { } } - fn partition_address(&self) -> usize { + fn partition_identifier(&self) -> u32 { 0x60000 } @@ -71,20 +70,6 @@ impl UpgradeStorage for BufferUpgradeStorage { PARTITION_LENGTH } - fn read_metadata(&self) -> StorageResult<&[u8]> { - Ok(&self.metadata[..]) - } - - fn write_metadata(&mut self, data: &[u8]) -> StorageResult<()> { - if data.len() <= METADATA_LENGTH { - self.metadata.copy_from_slice(&[0xff; METADATA_LENGTH]); - self.metadata[..data.len()].copy_from_slice(data); - Ok(()) - } else { - Err(StorageError::OutOfBounds) - } - } - fn running_firmware_version(&self) -> u64 { 0 } @@ -130,23 +115,7 @@ mod tests { #[test] fn partition_slice() { let storage = BufferUpgradeStorage::new().unwrap(); - assert_eq!(storage.partition_address(), 0x60000); + assert_eq!(storage.partition_identifier(), 0x60000); assert_eq!(storage.partition_length(), PARTITION_LENGTH); } - - #[test] - fn read_write_metadata() { - let mut storage = BufferUpgradeStorage::new().unwrap(); - assert_eq!(storage.read_metadata().unwrap(), &[0xFF; METADATA_LENGTH]); - assert!(storage.write_metadata(&[0x88, 0x88]).is_ok()); - assert_eq!( - storage.write_metadata(&[0x88; METADATA_LENGTH + 1]), - Err(StorageError::OutOfBounds) - ); - let new_metadata = storage.read_metadata().unwrap(); - assert_eq!(&new_metadata[0..2], &[0x88, 0x88]); - assert_eq!(&new_metadata[2..], &[0xFF; METADATA_LENGTH - 2]); - assert!(storage.write_metadata(&[]).is_ok()); - assert_eq!(storage.read_metadata().unwrap(), &[0xFF; METADATA_LENGTH]); - } } diff --git a/src/env/tock/storage.rs b/src/env/tock/storage.rs index 1c349b5..3d78cc8 100644 --- a/src/env/tock/storage.rs +++ b/src/env/tock/storage.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::api::upgrade_storage::helper::{find_slice, is_aligned, ModRange}; +use crate::api::upgrade_storage::helper::{find_slice, is_aligned, ModRange, Partition}; use crate::api::upgrade_storage::UpgradeStorage; use alloc::borrow::Cow; use alloc::vec::Vec; @@ -231,16 +231,17 @@ impl Storage for TockStorage { pub struct TockUpgradeStorage { page_size: usize, - partition: ModRange, + partition: Partition, metadata: ModRange, running_metadata: ModRange, + identifier: u32, } impl TockUpgradeStorage { // 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_A: usize = 0x20000; const PARTITION_ADDRESS_B: usize = 0x60000; /// Provides access to the other upgrade partition and metadata if available. @@ -260,13 +261,15 @@ impl TockUpgradeStorage { pub fn new() -> StorageResult { let mut locations = TockUpgradeStorage { page_size: get_info(command_nr::get_info_nr::PAGE_SIZE, 0)?, - partition: ModRange::new_empty(), + partition: Partition::new(), metadata: ModRange::new_empty(), running_metadata: ModRange::new_empty(), + identifier: Self::PARTITION_ADDRESS_A as u32, }; if !locations.page_size.is_power_of_two() { return Err(StorageError::CustomError); } + let mut firmware_range = ModRange::new_empty(); for i in 0..memop(memop_nr::STORAGE_CNT, 0)? { let storage_type = memop(memop_nr::STORAGE_TYPE, i)?; if !matches!(storage_type, storage_type::PARTITION) { @@ -286,21 +289,27 @@ impl TockUpgradeStorage { ModRange::new(range.start() + locations.page_size, locations.page_size); } _ => { - locations.partition = locations - .partition - .append(range) - .ok_or(StorageError::NotAligned)? + if !firmware_range.append(&range) { + return Err(StorageError::NotAligned); + } } } } - if locations.partition.is_empty() + if firmware_range.is_empty() || locations.metadata.is_empty() || locations.running_metadata.is_empty() { return Err(StorageError::CustomError); } - if locations.partition.start() == Self::PARTITION_ADDRESS_B { + if firmware_range.start() == Self::PARTITION_ADDRESS_B { core::mem::swap(&mut locations.metadata, &mut locations.running_metadata); + locations.identifier = Self::PARTITION_ADDRESS_B as u32; + } + if !locations.partition.append(locations.metadata.clone()) { + return Err(StorageError::NotAligned); + } + if !locations.partition.append(firmware_range) { + return Err(StorageError::NotAligned); } Ok(locations) } @@ -308,6 +317,38 @@ impl TockUpgradeStorage { fn is_page_aligned(&self, x: usize) -> bool { is_aligned(self.page_size, x) } + + /// Returns whether the metadata is contained in this range or not. + /// + /// Assumes that metadata is written in one call per range. If the metadata is only partially + /// contained, returns an error. + fn contains_metadata(&self, checked_range: &ModRange) -> StorageResult { + if checked_range.intersects_range(&self.metadata) { + if checked_range.contains_range(&self.metadata) { + Ok(true) + } else { + Err(StorageError::NotAligned) + } + } else { + Ok(false) + } + } + + /// Checks if the metadata's hash matches the partition's content. + fn check_partition_hash(&self, metadata: &[u8]) -> StorageResult<()> { + let start_address = self.metadata.start() + METADATA_SIGN_OFFSET; + let mut hasher = Sha256::new(); + for range in self.partition.ranges_from(start_address) { + let partition_slice = unsafe { read_slice(range.start(), range.length()) }; + // The hash implementation handles this in chunks, so no memory issues. + hasher.update(partition_slice); + } + let computed_hash = hasher.finalize(); + if &computed_hash != parse_metadata_hash(metadata) { + return Err(StorageError::CustomError); + } + Ok(()) + } } impl UpgradeStorage for TockUpgradeStorage { @@ -315,11 +356,7 @@ impl UpgradeStorage for TockUpgradeStorage { if length == 0 { return Err(StorageError::OutOfBounds); } - let address = self.partition.start() + offset; - if self - .partition - .contains_range(&ModRange::new(address, length)) - { + if let Some(address) = self.partition.find_address(offset, length) { Ok(unsafe { read_slice(address, length) }) } else { Err(StorageError::OutOfBounds) @@ -330,44 +367,38 @@ impl UpgradeStorage for TockUpgradeStorage { if data.is_empty() { return Err(StorageError::OutOfBounds); } - let address = self.partition.start() + offset; + let address = self + .partition + .find_address(offset, data.len()) + .ok_or(StorageError::OutOfBounds)?; let write_range = ModRange::new(address, data.len()); - if self.partition.contains_range(&write_range) { - // Erases all pages that have their first byte in the write range. - // Since we expect calls in order, we don't want to erase half-written pages. - for address in write_range.aligned_iter(self.page_size) { - erase_page(address, self.page_size)?; - } - write_slice(address, data) - } else { - Err(StorageError::OutOfBounds) + if self.contains_metadata(&write_range)? { + let new_metadata = &data[self.metadata.start() - address..][..self.metadata.length()]; + check_metadata(self, UPGRADE_PUBLIC_KEY, new_metadata)?; } + + // Erases all pages that have their first byte in the write range. + // Since we expect calls in order, we don't want to erase half-written pages. + for address in write_range.aligned_iter(self.page_size) { + erase_page(address, self.page_size)?; + } + write_slice(address, data)?; + // Case: Last slice is written. + if data.len() == self.partition_length() - offset { + let metadata = unsafe { read_slice(self.metadata.start(), self.metadata.length()) }; + self.check_partition_hash(&metadata)?; + } + Ok(()) } - fn partition_address(&self) -> usize { - self.partition.start() + fn partition_identifier(&self) -> u32 { + self.identifier } fn partition_length(&self) -> usize { self.partition.length() } - fn read_metadata(&self) -> StorageResult<&[u8]> { - Ok(unsafe { read_slice(self.metadata.start(), self.metadata.length()) }) - } - - fn write_metadata(&mut self, data: &[u8]) -> StorageResult<()> { - if data.len() != self.metadata.length() { - return Err(StorageError::CustomError); - } - // Compares the hash inside the metadata to the actual hash. - parse_metadata(self, UPGRADE_PUBLIC_KEY, &data)?; - for address in self.metadata.aligned_iter(self.page_size) { - erase_page(address, self.page_size)?; - } - write_slice(self.metadata.start(), data) - } - fn running_firmware_version(&self) -> u64 { let running_metadata = unsafe { read_slice( @@ -389,8 +420,9 @@ impl UpgradeStorage for TockUpgradeStorage { /// - 4 B partition address in little endian encoding /// written at METADATA_SIGN_OFFSET. /// -/// Checks hash and signature correctness, and whether the partition offset matches. -fn parse_metadata( +/// Checks signature correctness against the hash, and whether the partition offset matches. +/// Whether the hash matches the partition content is not tested here! +fn check_metadata( upgrade_locations: &impl UpgradeStorage, public_key_bytes: &[u8], metadata: &[u8], @@ -406,29 +438,23 @@ fn parse_metadata( } 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); - } - - // The hash implementation handles this in chunks, so no memory issues. - let partition_slice = - upgrade_locations.read_partition(0, upgrade_locations.partition_length())?; - let mut hasher = Sha256::new(); - hasher.update(&metadata[METADATA_SIGN_OFFSET..]); - hasher.update(partition_slice); - let computed_hash = hasher.finalize(); - if &computed_hash != array_ref!(metadata, 0, 32) { + if metadata_address != upgrade_locations.partition_identifier() { return Err(StorageError::CustomError); } verify_signature( array_ref!(metadata, 32, 64), public_key_bytes, - &computed_hash, + parse_metadata_hash(metadata), )?; Ok(()) } +/// Parses the metadata, returns the hash. +fn parse_metadata_hash(data: &[u8]) -> &[u8; 32] { + array_ref!(data, 0, 32) +} + /// Parses the metadata, returns the firmware version. fn parse_metadata_version(data: &[u8]) -> u64 { LittleEndian::read_u64(&data[METADATA_SIGN_OFFSET..][..8]) @@ -459,7 +485,7 @@ mod test { use crate::env::Env; #[test] - fn test_parse_metadata() { + fn test_check_metadata() { let mut env = TestEnv::new(); let private_key = crypto::ecdsa::SecKey::gensk(env.rng()); let upgrade_locations = env.upgrade_storage().unwrap(); @@ -489,42 +515,37 @@ mod test { public_key.to_bytes_uncompressed(&mut public_key_bytes); assert_eq!( - parse_metadata(upgrade_locations, &public_key_bytes, &metadata), + check_metadata(upgrade_locations, &public_key_bytes, &metadata), Ok(()) ); // Manipulating the partition address fails. - metadata[METADATA_SIGN_OFFSET] = 0x88; + metadata[METADATA_SIGN_OFFSET + 8] = 0x88; assert_eq!( - parse_metadata(upgrade_locations, &public_key_bytes, &metadata), + check_metadata(upgrade_locations, &public_key_bytes, &metadata), Err(StorageError::CustomError) ); - metadata[METADATA_SIGN_OFFSET] = 0x00; - // Any manipulation of signed data fails. - metadata[METADATA_LEN - 1] = 0x88; + metadata[METADATA_SIGN_OFFSET + 8] = 0x00; + // Wrong metadata length fails. assert_eq!( - parse_metadata(upgrade_locations, &public_key_bytes, &metadata), + check_metadata( + upgrade_locations, + &public_key_bytes, + &metadata[..METADATA_LEN - 1] + ), Err(StorageError::CustomError) ); - metadata[METADATA_LEN - 1] = 0xFF; // Manipulating the hash fails. metadata[0] ^= 0x01; assert_eq!( - parse_metadata(upgrade_locations, &public_key_bytes, &metadata), + check_metadata(upgrade_locations, &public_key_bytes, &metadata), Err(StorageError::CustomError) ); metadata[0] ^= 0x01; // Manipulating the signature fails. metadata[32] ^= 0x01; assert_eq!( - parse_metadata(upgrade_locations, &public_key_bytes, &metadata), - Err(StorageError::CustomError) - ); - metadata[32] ^= 0x01; - // Manipulating the partition data fails. - upgrade_locations.write_partition(0, &[0x88; 1]).unwrap(); - assert_eq!( - parse_metadata(upgrade_locations, &public_key_bytes, &metadata), + check_metadata(upgrade_locations, &public_key_bytes, &metadata), Err(StorageError::CustomError) ); } diff --git a/tools/deploy_partition.py b/tools/deploy_partition.py index 82ad2b3..d33f6af 100755 --- a/tools/deploy_partition.py +++ b/tools/deploy_partition.py @@ -181,6 +181,7 @@ def main(args): priv_key = load_priv_key(args.priv_key) metadata = create_metadata(firmware_image, partition_address, args.version, priv_key) + partition = metadata + firmware_image if args.use_vendor_hid: patcher = patch.object(hid.base, "FIDO_USAGE_PAGE", 0xFF00) @@ -200,7 +201,7 @@ def main(args): running_version = authenticator.get_info().firmware_version if args.version < running_version: - fatal(f"Can not write version {args.version} when version" + fatal(f"Can not write version {args.version} when version " f"{running_version} is running.") else: info(f"Running version: {running_version}") @@ -208,8 +209,8 @@ def main(args): try: check_info(partition_address, authenticator) offset = 0 - for offset in range(0, len(firmware_image), PAGE_SIZE): - page = firmware_image[offset:][:PAGE_SIZE] + for offset in range(0, len(partition), PAGE_SIZE): + page = partition[offset:][:PAGE_SIZE] info(f"Writing at offset 0x{offset:08X}...") cbor_data = {1: offset, 2: page, 3: hash_message(page)} authenticator.send_cbor( @@ -217,22 +218,14 @@ def main(args): data=cbor_data, ) - info("Writing metadata...") - # TODO Write the correct address when the metadata is transparent. - cbor_data = {2: metadata, 3: hash_message(metadata)} - authenticator.send_cbor( - OPENSK_VENDOR_UPGRADE, - data=cbor_data, - ) - except ctap.CtapError as ex: message = "Failed to upgrade OpenSK" if ex.code.value == ctap.CtapError.ERR.INVALID_COMMAND: error(f"{message} (unsupported command).") elif ex.code.value == ctap.CtapError.ERR.INVALID_PARAMETER: - error(f"{message} (invalid parameter, maybe a wrong byte array size?).") + error(f"{message} (invalid parameter).") elif ex.code.value == ctap.CtapError.ERR.INTEGRITY_FAILURE: - error(f"{message} (metadata parsing failed).") + error(f"{message} (data hash does not match slice).") elif ex.code.value == 0xF2: # VENDOR_INTERNAL_ERROR error(f"{message} (internal conditions not met).") elif ex.code.value == 0xF3: # VENDOR_HARDWARE_FAILURE