From 3fd77a99c010dc3fb99cde2ab112a97e6b6805c5 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Mon, 8 Apr 2024 14:56:58 +0200 Subject: [PATCH] Addresses comments with different parameter typing --- libraries/opensk/src/api/persist.rs | 26 +++++++++++--------- libraries/opensk/src/ctap/large_blobs.rs | 31 +++++++++++++----------- libraries/opensk/src/ctap/mod.rs | 2 +- 3 files changed, 32 insertions(+), 27 deletions(-) diff --git a/libraries/opensk/src/api/persist.rs b/libraries/opensk/src/api/persist.rs index 6caaec99..48d5cd8f 100644 --- a/libraries/opensk/src/api/persist.rs +++ b/libraries/opensk/src/api/persist.rs @@ -18,6 +18,7 @@ use crate::api::crypto::EC_FIELD_SIZE; use crate::ctap::secret::Secret; use crate::ctap::status_code::{Ctap2StatusCode, CtapResult}; use crate::ctap::PIN_AUTH_LENGTH; +use alloc::borrow::Cow; use alloc::boxed::Box; use alloc::vec::Vec; use core::cmp; @@ -27,6 +28,7 @@ use enum_iterator::IntoEnumIterator; pub type PersistIter<'a> = Box> + 'a>; pub type PersistCredentialIter<'a> = Box)>> + 'a>; +pub type LargeBlobBuffer = Vec; /// Stores data that persists across reboots. /// @@ -235,7 +237,7 @@ pub trait Persist { /// Prepares writing a new large blob. /// /// Returns a buffer that is returned to other API calls for potential usage. - fn init_large_blob(&mut self, expected_length: usize) -> CtapResult> { + fn init_large_blob(&mut self, expected_length: usize) -> CtapResult { Ok(Vec::with_capacity(expected_length)) } @@ -245,14 +247,14 @@ pub trait Persist { fn write_large_blob_chunk( &mut self, offset: usize, - chunk: &mut Vec, - buffer: &mut Vec, + chunk: &[u8], + buffer: &mut LargeBlobBuffer, ) -> CtapResult<()> { if buffer.len() != offset { // This should be caught on CTAP level. return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR); } - buffer.append(chunk); + buffer.extend_from_slice(chunk); Ok(()) } @@ -262,17 +264,17 @@ pub trait Persist { /// available. This includes cases of offset being beyond the stored array. /// /// The buffer is passed in when writing is in process. - fn get_large_blob( - &self, + fn get_large_blob<'a>( + &'a self, mut offset: usize, byte_count: usize, - buffer: Option<&Vec>, - ) -> CtapResult>> { + buffer: Option<&'a LargeBlobBuffer>, + ) -> CtapResult>> { if let Some(buffer) = buffer { let start = cmp::min(offset, buffer.len()); let end = offset.saturating_add(byte_count); let end = cmp::min(end, buffer.len()); - return Ok(Some(buffer[start..end].to_vec())); + return Ok(Some(Cow::from(&buffer[start..end]))); } let mut result = Vec::with_capacity(byte_count); for key in keys::LARGE_BLOB_SHARDS { @@ -288,16 +290,16 @@ pub trait Persist { } let end = cmp::min(end, value.len()); if end < offset { - return Ok(Some(result)); + return Ok(Some(Cow::from(result))); } result.extend(&value[offset..end]); offset = offset.saturating_sub(VALUE_LENGTH); } - Ok(Some(result)) + Ok(Some(Cow::from(result))) } /// Sets a byte vector as the serialized large blobs array. - fn commit_large_blob_array(&mut self, buffer: &Vec) -> CtapResult<()> { + fn commit_large_blob_array(&mut self, buffer: &LargeBlobBuffer) -> CtapResult<()> { debug_assert!(buffer.len() <= keys::LARGE_BLOB_SHARDS.len() * VALUE_LENGTH); let mut offset = 0; for key in keys::LARGE_BLOB_SHARDS { diff --git a/libraries/opensk/src/ctap/large_blobs.rs b/libraries/opensk/src/ctap/large_blobs.rs index 6c5973f6..5026a8e3 100644 --- a/libraries/opensk/src/ctap/large_blobs.rs +++ b/libraries/opensk/src/ctap/large_blobs.rs @@ -66,7 +66,7 @@ impl LargeBlobState { ))); } - if let Some(mut set) = set { + if let Some(set) = set { if set.len() > max_fragment_size { return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_LENGTH); } @@ -112,7 +112,7 @@ impl LargeBlobState { } let received_length = set.len(); env.persist() - .write_large_blob_chunk(offset, &mut set, &mut self.buffer)?; + .write_large_blob_chunk(offset, &set, &mut self.buffer)?; self.expected_next_offset += received_length; if self.expected_next_offset == self.expected_length { const CHUNK_SIZE: usize = 1024; @@ -132,7 +132,7 @@ impl LargeBlobState { .persist() .get_large_blob(buffer_hash_index, TRUNCATED_HASH_LEN, Some(&self.buffer))? .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?; - if computed_hash[..TRUNCATED_HASH_LEN] != written_hash { + if computed_hash[..TRUNCATED_HASH_LEN] != written_hash[..] { return Err(Ctap2StatusCode::CTAP2_ERR_INTEGRITY_FAILURE); } env.persist().commit_large_blob_array(&self.buffer)?; @@ -163,17 +163,20 @@ fn get_large_blob_array( byte_count: usize, ) -> CtapResult> { let output = env.persist().get_large_blob(offset, byte_count, None)?; - Ok(output.unwrap_or_else(|| { - const EMPTY_LARGE_BLOB: [u8; 17] = [ - 0x80, 0x76, 0xBE, 0x8B, 0x52, 0x8D, 0x00, 0x75, 0xF7, 0xAA, 0xE9, 0x8D, 0x6F, 0xA5, - 0x7A, 0x6D, 0x3C, - ]; - let last_index = cmp::min(EMPTY_LARGE_BLOB.len(), offset.saturating_add(byte_count)); - EMPTY_LARGE_BLOB - .get(offset..last_index) - .unwrap_or_default() - .to_vec() - })) + Ok(match output { + Some(data) => data.into(), + None => { + const EMPTY_LARGE_BLOB: [u8; 17] = [ + 0x80, 0x76, 0xBE, 0x8B, 0x52, 0x8D, 0x00, 0x75, 0xF7, 0xAA, 0xE9, 0x8D, 0x6F, 0xA5, + 0x7A, 0x6D, 0x3C, + ]; + let last_index = cmp::min(EMPTY_LARGE_BLOB.len(), offset.saturating_add(byte_count)); + EMPTY_LARGE_BLOB + .get(offset..last_index) + .unwrap_or_default() + .to_vec() + } + }) } #[cfg(test)] diff --git a/libraries/opensk/src/ctap/mod.rs b/libraries/opensk/src/ctap/mod.rs index 4909333b..ae2c56d0 100644 --- a/libraries/opensk/src/ctap/mod.rs +++ b/libraries/opensk/src/ctap/mod.rs @@ -561,7 +561,7 @@ impl StatefulPermission { channel, ); } - if let Some(StatefulCommand::LargeBlob(ref mut large_blob_state)) = &mut self.command_type { + if let Some(StatefulCommand::LargeBlob(large_blob_state)) = &mut self.command_type { large_blob_state } else { unreachable!();