kmwebnet/OpenSK
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Simplifies Env upgrade API (#551)

Browse Source 
* removes read_partition and partition_length from upgrade API

* renames partition to bundle, also data type change from slice to Vec

* removes hash from Env API

* fixes comment
This commit is contained in:
kaczmarczyck
2022-10-03 16:33:34 +02:00
committed by GitHub
parent 6610a29a67
commit 3c28ff49ee
5 changed files with 62 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
src/api/upgrade_storage/mod.rs
View File

@@ -12,41 +12,27 @@
// See the License for the specific language governing permissions and // See the License for the specific language governing permissions and
// limitations under the License. // limitations under the License.
use alloc::vec::Vec;
use persistent_store::StorageResult; use persistent_store::StorageResult;
pub(crate) mod helper; pub(crate) mod helper;
/// Accessors to storage locations used for upgrading from a CTAP command. /// Accessors to storage locations used for upgrading from a CTAP command.
pub trait UpgradeStorage { pub trait UpgradeStorage {
/// Reads a slice of the partition, if within bounds. /// Processes the given data as part of an upgrade.
/// ///
/// The offset is relative to the start of the partition, excluding holes. The partition is /// The offset indicates the data location inside the bundle.
/// presented as one connected component. Therefore, the offset does not easily translate
/// to physical memory address address of the slice.
/// ///
/// # Errors /// # Errors
/// ///
/// Returns [`StorageError::OutOfBounds`] if the requested slice is not inside the partition. /// - Returns [`StorageError::OutOfBounds`] if the data does not fit.
fn read_partition(&self, offset: usize, length: usize) -> StorageResult<&[u8]>; /// - Returns [`StorageError::CustomError`] if any Metadata or other check fails.
fn write_bundle(&mut self, offset: usize, data: Vec<u8>) -> StorageResult<()>;
/// Writes the given data to the given offset address, if within bounds of the partition. /// Returns an identifier for the requested bundle.
///
/// 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::CustomError`] if any Metadata check fails.
fn write_partition(&mut self, offset: usize, data: &[u8]) -> StorageResult<()>;
/// Returns an identifier for the partition.
/// ///
/// Use this to determine whether you are writing to A or B. /// Use this to determine whether you are writing to A or B.
fn partition_identifier(&self) -> u32; fn bundle_identifier(&self) -> u32;
/// Returns the length of the partition.
fn partition_length(&self) -> usize;
/// Returns the currently running firmware version. /// Returns the currently running firmware version.
fn running_firmware_version(&self) -> u64; fn running_firmware_version(&self) -> u64;

18
src/ctap/command.rs
View File

@@ -594,7 +594,7 @@ impl TryFrom<cbor::Value> for AuthenticatorVendorConfigureParameters {
pub struct AuthenticatorVendorUpgradeParameters { pub struct AuthenticatorVendorUpgradeParameters {
pub offset: usize, pub offset: usize,
pub data: Vec<u8>, pub data: Vec<u8>,
pub hash: Vec<u8>, pub hash: [u8; 32],
} }
impl TryFrom<cbor::Value> for AuthenticatorVendorUpgradeParameters { impl TryFrom<cbor::Value> for AuthenticatorVendorUpgradeParameters {
@@ -610,7 +610,8 @@ impl TryFrom<cbor::Value> for AuthenticatorVendorUpgradeParameters {
} }
let offset = extract_unsigned(ok_or_missing(offset)?)? as usize; let offset = extract_unsigned(ok_or_missing(offset)?)? as usize;
let data = extract_byte_string(ok_or_missing(data)?)?; let data = extract_byte_string(ok_or_missing(data)?)?;
let hash = extract_byte_string(ok_or_missing(hash)?)?; let hash = <[u8; 32]>::try_from(extract_byte_string(ok_or_missing(hash)?)?)
.map_err(|_| Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)?;
Ok(AuthenticatorVendorUpgradeParameters { offset, data, hash }) Ok(AuthenticatorVendorUpgradeParameters { offset, data, hash })
} }
} }
@@ -1080,6 +1081,17 @@ mod test {
Err(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER) Err(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER)
); );
// Invalid hash size
let cbor_value = cbor_map! {
0x01 => 0x1000,
0x02 => [0xFF; 0x100],
0x03 => [0x44; 33],
};
assert_eq!(
AuthenticatorVendorUpgradeParameters::try_from(cbor_value),
Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)
);
// Missing hash // Missing hash
let cbor_value = cbor_map! { let cbor_value = cbor_map! {
0x01 => 0x1000, 0x01 => 0x1000,
@@ -1101,7 +1113,7 @@ mod test {
Ok(AuthenticatorVendorUpgradeParameters { Ok(AuthenticatorVendorUpgradeParameters {
offset: 0x1000, offset: 0x1000,
data: vec![0xFF; 0x100], data: vec![0xFF; 0x100],
hash: vec![0x44; 32], hash: [0x44; 32],
}) })
); );
} }

39
src/ctap/mod.rs
View File

@@ -1399,19 +1399,14 @@ impl CtapState {
params: AuthenticatorVendorUpgradeParameters, params: AuthenticatorVendorUpgradeParameters,
) -> Result<ResponseData, Ctap2StatusCode> { ) -> Result<ResponseData, Ctap2StatusCode> {
let AuthenticatorVendorUpgradeParameters { offset, data, hash } = params; let AuthenticatorVendorUpgradeParameters { offset, data, hash } = params;
let upgrade_locations = env let calculated_hash = Sha256::hash(&data);
.upgrade_storage() if hash != calculated_hash {
.ok_or(Ctap2StatusCode::CTAP1_ERR_INVALID_COMMAND)?;
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); return Err(Ctap2StatusCode::CTAP2_ERR_INTEGRITY_FAILURE);
} }
env.upgrade_storage()
.ok_or(Ctap2StatusCode::CTAP1_ERR_INVALID_COMMAND)?
.write_bundle(offset, data)
.map_err(|_| Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)?;
Ok(ResponseData::AuthenticatorVendorUpgrade) Ok(ResponseData::AuthenticatorVendorUpgrade)
} }
@@ -1424,7 +1419,7 @@ impl CtapState {
.ok_or(Ctap2StatusCode::CTAP1_ERR_INVALID_COMMAND)?; .ok_or(Ctap2StatusCode::CTAP1_ERR_INVALID_COMMAND)?;
Ok(ResponseData::AuthenticatorVendorUpgradeInfo( Ok(ResponseData::AuthenticatorVendorUpgradeInfo(
AuthenticatorVendorUpgradeInfoResponse { AuthenticatorVendorUpgradeInfoResponse {
info: upgrade_locations.partition_identifier(), info: upgrade_locations.bundle_identifier(),
}, },
)) ))
} }
@@ -3393,9 +3388,9 @@ mod test {
const METADATA_LEN: usize = 0x1000; const METADATA_LEN: usize = 0x1000;
let metadata = vec![0xFF; METADATA_LEN]; let metadata = vec![0xFF; METADATA_LEN];
let metadata_hash = Sha256::hash(&metadata).to_vec(); let metadata_hash = Sha256::hash(&metadata);
let data = vec![0xFF; 0x1000]; let data = vec![0xFF; 0x1000];
let hash = Sha256::hash(&data).to_vec(); let hash = Sha256::hash(&data);
// Write to partition. // Write to partition.
let response = ctap_state.process_vendor_upgrade( let response = ctap_state.process_vendor_upgrade(
@@ -3403,7 +3398,7 @@ mod test {
AuthenticatorVendorUpgradeParameters { AuthenticatorVendorUpgradeParameters {
offset: 0x20000, offset: 0x20000,
data: data.clone(), data: data.clone(),
hash: hash.clone(), hash,
}, },
); );
assert_eq!(response, Ok(ResponseData::AuthenticatorVendorUpgrade)); assert_eq!(response, Ok(ResponseData::AuthenticatorVendorUpgrade));
@@ -3414,7 +3409,7 @@ mod test {
AuthenticatorVendorUpgradeParameters { AuthenticatorVendorUpgradeParameters {
offset: 0, offset: 0,
data: metadata.clone(), data: metadata.clone(),
hash: metadata_hash.clone(), hash: metadata_hash,
}, },
); );
assert_eq!(response, Ok(ResponseData::AuthenticatorVendorUpgrade)); assert_eq!(response, Ok(ResponseData::AuthenticatorVendorUpgrade));
@@ -3425,7 +3420,7 @@ mod test {
AuthenticatorVendorUpgradeParameters { AuthenticatorVendorUpgradeParameters {
offset: METADATA_LEN, offset: METADATA_LEN,
data: data.clone(), data: data.clone(),
hash: hash.clone(), hash,
}, },
); );
assert_eq!(response, Ok(ResponseData::AuthenticatorVendorUpgrade)); assert_eq!(response, Ok(ResponseData::AuthenticatorVendorUpgrade));
@@ -3439,7 +3434,7 @@ mod test {
hash: metadata_hash, hash: metadata_hash,
}, },
); );
assert_eq!(response, Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)); assert_eq!(response, Err(Ctap2StatusCode::CTAP2_ERR_INTEGRITY_FAILURE));
// Write outside of the partition. // Write outside of the partition.
let response = ctap_state.process_vendor_upgrade( let response = ctap_state.process_vendor_upgrade(
@@ -3458,7 +3453,7 @@ mod test {
AuthenticatorVendorUpgradeParameters { AuthenticatorVendorUpgradeParameters {
offset: 0x20000, offset: 0x20000,
data, data,
hash: [0xEE; 32].to_vec(), hash: [0xEE; 32],
}, },
); );
assert_eq!(response, Err(Ctap2StatusCode::CTAP2_ERR_INTEGRITY_FAILURE)); assert_eq!(response, Err(Ctap2StatusCode::CTAP2_ERR_INTEGRITY_FAILURE));
@@ -3471,7 +3466,7 @@ mod test {
let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
let data = vec![0xFF; 0x1000]; let data = vec![0xFF; 0x1000];
let hash = Sha256::hash(&data).to_vec(); let hash = Sha256::hash(&data);
let response = ctap_state.process_vendor_upgrade( let response = ctap_state.process_vendor_upgrade(
&mut env, &mut env,
AuthenticatorVendorUpgradeParameters { AuthenticatorVendorUpgradeParameters {
@@ -3487,14 +3482,14 @@ mod test {
fn test_vendor_upgrade_info() { fn test_vendor_upgrade_info() {
let mut env = TestEnv::new(); let mut env = TestEnv::new();
let ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); let ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
let partition_identifier = env.upgrade_storage().unwrap().partition_identifier(); let bundle_identifier = env.upgrade_storage().unwrap().bundle_identifier();
let upgrade_info_reponse = ctap_state.process_vendor_upgrade_info(&mut env); let upgrade_info_reponse = ctap_state.process_vendor_upgrade_info(&mut env);
assert_eq!( assert_eq!(
upgrade_info_reponse, upgrade_info_reponse,
Ok(ResponseData::AuthenticatorVendorUpgradeInfo( Ok(ResponseData::AuthenticatorVendorUpgradeInfo(
AuthenticatorVendorUpgradeInfoResponse { AuthenticatorVendorUpgradeInfoResponse {
info: partition_identifier, info: bundle_identifier,
} }
)) ))
); );

28
src/env/test/upgrade_storage.rs vendored
View File

@@ -31,9 +31,8 @@ impl BufferUpgradeStorage {
partition: vec![0xff; PARTITION_LENGTH].into_boxed_slice(), partition: vec![0xff; PARTITION_LENGTH].into_boxed_slice(),
}) })
} }
}
impl UpgradeStorage for BufferUpgradeStorage { #[cfg(test)]
fn read_partition(&self, offset: usize, length: usize) -> StorageResult<&[u8]> { fn read_partition(&self, offset: usize, length: usize) -> StorageResult<&[u8]> {
if length == 0 { if length == 0 {
return Err(StorageError::OutOfBounds); return Err(StorageError::OutOfBounds);
@@ -45,8 +44,10 @@ impl UpgradeStorage for BufferUpgradeStorage {
Err(StorageError::OutOfBounds) Err(StorageError::OutOfBounds)
} }
} }
}
fn write_partition(&mut self, offset: usize, data: &[u8]) -> StorageResult<()> { impl UpgradeStorage for BufferUpgradeStorage {
fn write_bundle(&mut self, offset: usize, data: Vec<u8>) -> StorageResult<()> {
if offset == 0 && data.len() != METADATA_LENGTH { if offset == 0 && data.len() != METADATA_LENGTH {
return Err(StorageError::OutOfBounds); return Err(StorageError::OutOfBounds);
} }
@@ -55,21 +56,17 @@ impl UpgradeStorage for BufferUpgradeStorage {
} }
let partition_range = ModRange::new(0, self.partition.len()); let partition_range = ModRange::new(0, self.partition.len());
if partition_range.contains_range(&ModRange::new(offset, data.len())) { if partition_range.contains_range(&ModRange::new(offset, data.len())) {
self.partition[offset..][..data.len()].copy_from_slice(data); self.partition[offset..][..data.len()].copy_from_slice(&data);
Ok(()) Ok(())
} else { } else {
Err(StorageError::OutOfBounds) Err(StorageError::OutOfBounds)
} }
} }
fn partition_identifier(&self) -> u32 { fn bundle_identifier(&self) -> u32 {
0x60000 0x60000
} }
fn partition_length(&self) -> usize {
PARTITION_LENGTH
}
fn running_firmware_version(&self) -> u64 { fn running_firmware_version(&self) -> u64 {
0 0
} }
@@ -80,13 +77,13 @@ mod tests {
use super::*; use super::*;
#[test] #[test]
fn read_write_partition() { fn read_write_bundle() {
let mut storage = BufferUpgradeStorage::new().unwrap(); let mut storage = BufferUpgradeStorage::new().unwrap();
assert_eq!(storage.read_partition(0, 2).unwrap(), &[0xFF, 0xFF]); assert_eq!(storage.read_partition(0, 2).unwrap(), &[0xFF, 0xFF]);
assert!(storage.write_partition(1, &[0x88, 0x88]).is_ok()); assert!(storage.write_bundle(1, vec![0x88, 0x88]).is_ok());
assert_eq!(storage.read_partition(0, 2).unwrap(), &[0xFF, 0x88]); assert_eq!(storage.read_partition(0, 2).unwrap(), &[0xFF, 0x88]);
assert_eq!( assert_eq!(
storage.write_partition(PARTITION_LENGTH - 1, &[0x88, 0x88]), storage.write_bundle(PARTITION_LENGTH - 1, vec![0x88, 0x88],),
Err(StorageError::OutOfBounds) Err(StorageError::OutOfBounds)
); );
assert_eq!( assert_eq!(
@@ -98,11 +95,11 @@ mod tests {
Err(StorageError::OutOfBounds) Err(StorageError::OutOfBounds)
); );
assert_eq!( assert_eq!(
storage.write_partition(4, &[]), storage.write_bundle(4, vec![]),
Err(StorageError::OutOfBounds) Err(StorageError::OutOfBounds)
); );
assert_eq!( assert_eq!(
storage.write_partition(PARTITION_LENGTH + 4, &[]), storage.write_bundle(PARTITION_LENGTH + 4, vec![]),
Err(StorageError::OutOfBounds) Err(StorageError::OutOfBounds)
); );
assert_eq!(storage.read_partition(4, 0), Err(StorageError::OutOfBounds)); assert_eq!(storage.read_partition(4, 0), Err(StorageError::OutOfBounds));
@@ -115,7 +112,6 @@ mod tests {
#[test] #[test]
fn partition_slice() { fn partition_slice() {
let storage = BufferUpgradeStorage::new().unwrap(); let storage = BufferUpgradeStorage::new().unwrap();
assert_eq!(storage.partition_identifier(), 0x60000); assert_eq!(storage.bundle_identifier(), 0x60000);
assert_eq!(storage.partition_length(), PARTITION_LENGTH);
} }
} }

36
src/env/tock/storage.rs vendored
View File

@@ -352,18 +352,7 @@ impl TockUpgradeStorage {
} }
impl UpgradeStorage for TockUpgradeStorage { impl UpgradeStorage for TockUpgradeStorage {
fn read_partition(&self, offset: usize, length: usize) -> StorageResult<&[u8]> { fn write_bundle(&mut self, offset: usize, data: Vec<u8>) -> StorageResult<()> {
if length == 0 {
return Err(StorageError::OutOfBounds);
}
if let Some(address) = self.partition.find_address(offset, length) {
Ok(unsafe { read_slice(address, length) })
} else {
Err(StorageError::OutOfBounds)
}
}
fn write_partition(&mut self, offset: usize, data: &[u8]) -> StorageResult<()> {
if data.is_empty() { if data.is_empty() {
return Err(StorageError::OutOfBounds); return Err(StorageError::OutOfBounds);
} }
@@ -382,23 +371,23 @@ impl UpgradeStorage for TockUpgradeStorage {
for address in write_range.aligned_iter(self.page_size) { for address in write_range.aligned_iter(self.page_size) {
erase_page(address, self.page_size)?; erase_page(address, self.page_size)?;
} }
write_slice(address, data)?; write_slice(address, &data)?;
let written_slice = unsafe { read_slice(address, data.len()) };
if written_slice != data {
return Err(StorageError::CustomError);
}
// Case: Last slice is written. // Case: Last slice is written.
if data.len() == self.partition_length() - offset { if data.len() == self.partition.length() - offset {
let metadata = unsafe { read_slice(self.metadata.start(), self.metadata.length()) }; let metadata = unsafe { read_slice(self.metadata.start(), self.metadata.length()) };
self.check_partition_hash(&metadata)?; self.check_partition_hash(&metadata)?;
} }
Ok(()) Ok(())
} }
fn partition_identifier(&self) -> u32 { fn bundle_identifier(&self) -> u32 {
self.identifier self.identifier
} }
fn partition_length(&self) -> usize {
self.partition.length()
}
fn running_firmware_version(&self) -> u64 { fn running_firmware_version(&self) -> u64 {
let running_metadata = unsafe { let running_metadata = unsafe {
read_slice( read_slice(
@@ -438,7 +427,7 @@ fn check_metadata(
} }
let metadata_address = LittleEndian::read_u32(&metadata[METADATA_SIGN_OFFSET + 8..][..4]); let metadata_address = LittleEndian::read_u32(&metadata[METADATA_SIGN_OFFSET + 8..][..4]);
if metadata_address != upgrade_locations.partition_identifier() { if metadata_address != upgrade_locations.bundle_identifier() {
return Err(StorageError::CustomError); return Err(StorageError::CustomError);
} }
@@ -495,13 +484,8 @@ mod test {
let mut metadata = vec![0xFF; METADATA_LEN]; let mut metadata = vec![0xFF; METADATA_LEN];
LittleEndian::write_u32(&mut metadata[METADATA_SIGN_OFFSET + 8..][..4], 0x60000); LittleEndian::write_u32(&mut metadata[METADATA_SIGN_OFFSET + 8..][..4], 0x60000);
let partition_length = upgrade_locations.partition_length();
let mut signed_over_data = metadata[METADATA_SIGN_OFFSET..].to_vec(); let mut signed_over_data = metadata[METADATA_SIGN_OFFSET..].to_vec();
signed_over_data.extend( signed_over_data.extend(&[0xFF; 0x20000]);
upgrade_locations
.read_partition(0, partition_length)
.unwrap(),
);
let signed_hash = Sha256::hash(&signed_over_data); let signed_hash = Sha256::hash(&signed_over_data);
metadata[..32].copy_from_slice(&signed_hash); metadata[..32].copy_from_slice(&signed_hash);