From 87be49e36b4de1d425257a280c3e2f4e8dbf371e Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Fri, 5 Apr 2024 11:27:28 +0200 Subject: [PATCH 1/3] LargeBlob trait supports buffering choice Also makes LargeBlob a proper stateful command that loses state when interleaved. --- libraries/opensk/src/api/persist.rs | 53 ++-- libraries/opensk/src/ctap/command.rs | 34 +-- libraries/opensk/src/ctap/large_blobs.rs | 303 ++++++++++++++++++++--- libraries/opensk/src/ctap/mod.rs | 116 ++++++++- libraries/opensk/src/ctap/storage.rs | 96 ------- 5 files changed, 422 insertions(+), 180 deletions(-) diff --git a/libraries/opensk/src/api/persist.rs b/libraries/opensk/src/api/persist.rs index 97f24274..6caaec99 100644 --- a/libraries/opensk/src/api/persist.rs +++ b/libraries/opensk/src/api/persist.rs @@ -232,27 +232,48 @@ pub trait Persist { self.insert(keys::MIN_PIN_LENGTH_RP_IDS, min_pin_length_rp_ids_bytes) } - // TODO rework LargeBlob - // Problem 1: Env should be allowed to choose whether to buffer in memory or persist - // Otherwise small RAM devices have limited large blog size. - // Problem 2: LargeBlob is a stateful command, but doesn't use the safeguard and infrastructure - // of Stateful command. It has to be migrated there. - // While doing that, check if PinUvAuthToken timers and StatefulCommand timeouts are working - // together correctly. + /// 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> { + Ok(Vec::with_capacity(expected_length)) + } + + /// Writes a large blob chunk to the buffer. + /// + /// This can be the passed in buffer, or a custom solution. + fn write_large_blob_chunk( + &mut self, + offset: usize, + chunk: &mut Vec, + buffer: &mut Vec, + ) -> CtapResult<()> { + if buffer.len() != offset { + // This should be caught on CTAP level. + return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR); + } + buffer.append(chunk); + Ok(()) + } + /// Reads the byte vector stored as the serialized large blobs array. /// /// If too few bytes exist at that offset, return the maximum number /// available. This includes cases of offset being beyond the stored array. /// - /// If no large blob is committed to the store, get responds as if an empty - /// CBOR array (0x80) was written, together with the 16 byte prefix of its - /// SHA256, to a total length of 17 byte (which is the shortest legitimate - /// large blob entry possible). - fn get_large_blob_array( + /// The buffer is passed in when writing is in process. + fn get_large_blob( &self, mut offset: usize, byte_count: usize, + buffer: Option<&Vec>, ) -> 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())); + } let mut result = Vec::with_capacity(byte_count); for key in keys::LARGE_BLOB_SHARDS { if offset >= VALUE_LENGTH { @@ -276,12 +297,12 @@ pub trait Persist { } /// Sets a byte vector as the serialized large blobs array. - fn commit_large_blob_array(&mut self, large_blob_array: &[u8]) -> CtapResult<()> { - debug_assert!(large_blob_array.len() <= keys::LARGE_BLOB_SHARDS.len() * VALUE_LENGTH); + fn commit_large_blob_array(&mut self, buffer: &Vec) -> CtapResult<()> { + debug_assert!(buffer.len() <= keys::LARGE_BLOB_SHARDS.len() * VALUE_LENGTH); let mut offset = 0; for key in keys::LARGE_BLOB_SHARDS { - let cur_len = cmp::min(large_blob_array.len().saturating_sub(offset), VALUE_LENGTH); - let slice = &large_blob_array[offset..][..cur_len]; + let cur_len = cmp::min(buffer.len().saturating_sub(offset), VALUE_LENGTH); + let slice = &buffer[offset..][..cur_len]; if slice.is_empty() { self.remove(key)?; } else { diff --git a/libraries/opensk/src/ctap/command.rs b/libraries/opensk/src/ctap/command.rs index 9bc6fbc4..7d4820f0 100644 --- a/libraries/opensk/src/ctap/command.rs +++ b/libraries/opensk/src/ctap/command.rs @@ -32,9 +32,6 @@ use core::convert::TryFrom; use sk_cbor as cbor; use sk_cbor::destructure_cbor_map; -// This constant is a consequence of the structure of messages. -const MIN_LARGE_BLOB_LEN: usize = 17; - // CTAP specification (version 20190130) section 6.1 #[derive(Debug, PartialEq, Eq)] #[allow(clippy::enum_variant_names)] @@ -399,10 +396,7 @@ impl TryFrom for AuthenticatorLargeBlobsParameters { .map(PinUvAuthProtocol::try_from) .transpose()?; - if get.is_none() && set.is_none() { - return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); - } - if get.is_some() && set.is_some() { + if get.is_some() == set.is_some() { return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); } if get.is_some() @@ -410,16 +404,7 @@ impl TryFrom for AuthenticatorLargeBlobsParameters { { return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); } - if set.is_some() && offset == 0 { - match length { - None => return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER), - Some(len) if len < MIN_LARGE_BLOB_LEN => { - return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER) - } - Some(_) => (), - } - } - if set.is_some() && offset != 0 && length.is_some() { + if set.is_some() && ((offset == 0) != length.is_some()) { return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); } @@ -761,6 +746,8 @@ mod test { #[test] fn test_from_cbor_large_blobs_parameters() { + const MIN_LARGE_BLOB_LEN: usize = 17; + // successful get let cbor_value = cbor_map! { 0x01 => 2, @@ -875,19 +862,6 @@ mod test { Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER) ); - // failing with length smaller than minimum - let cbor_value = cbor_map! { - 0x02 => vec! [0x5E], - 0x03 => 0, - 0x04 => MIN_LARGE_BLOB_LEN as u64 - 1, - 0x05 => vec! [0xA9], - 0x06 => 1, - }; - assert_eq!( - AuthenticatorLargeBlobsParameters::try_from(cbor_value), - Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER) - ); - // failing with non-zero offset and length present let cbor_value = cbor_map! { 0x02 => vec! [0x5E], diff --git a/libraries/opensk/src/ctap/large_blobs.rs b/libraries/opensk/src/ctap/large_blobs.rs index caa0a20a..6c5973f6 100644 --- a/libraries/opensk/src/ctap/large_blobs.rs +++ b/libraries/opensk/src/ctap/large_blobs.rs @@ -15,7 +15,7 @@ use super::client_pin::{ClientPin, PinPermission}; use super::command::AuthenticatorLargeBlobsParameters; use super::response::{AuthenticatorLargeBlobsResponse, ResponseData}; -use super::status_code::Ctap2StatusCode; +use super::status_code::{Ctap2StatusCode, CtapResult}; use crate::api::crypto::sha256::Sha256; use crate::api::customization::Customization; use crate::api::persist::Persist; @@ -24,33 +24,27 @@ use crate::env::{Env, Sha}; use alloc::vec; use alloc::vec::Vec; use byteorder::{ByteOrder, LittleEndian}; +use core::cmp; /// The length of the truncated hash that is appended to the large blob data. const TRUNCATED_HASH_LEN: usize = 16; -pub struct LargeBlobs { +#[derive(Default)] +pub struct LargeBlobState { buffer: Vec, expected_length: usize, expected_next_offset: usize, } /// Implements the logic for the AuthenticatorLargeBlobs command and keeps its state. -impl LargeBlobs { - pub fn new() -> LargeBlobs { - LargeBlobs { - buffer: Vec::new(), - expected_length: 0, - expected_next_offset: 0, - } - } - +impl LargeBlobState { /// Process the large blob command. pub fn process_command( &mut self, env: &mut E, client_pin: &mut ClientPin, large_blobs_params: AuthenticatorLargeBlobsParameters, - ) -> Result { + ) -> CtapResult { let AuthenticatorLargeBlobsParameters { get, set, @@ -66,7 +60,7 @@ impl LargeBlobs { if get > max_fragment_size || offset.checked_add(get).is_none() { return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_LENGTH); } - let config = storage::get_large_blob_array(env, offset, get)?; + let config = get_large_blob_array(env, offset, get)?; return Ok(ResponseData::AuthenticatorLargeBlobs(Some( AuthenticatorLargeBlobsResponse { config }, ))); @@ -82,7 +76,12 @@ impl LargeBlobs { if self.expected_length > env.customization().max_large_blob_array_size() { return Err(Ctap2StatusCode::CTAP2_ERR_LARGE_BLOB_STORAGE_FULL); } + if self.expected_length <= TRUNCATED_HASH_LEN { + return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); + } self.expected_next_offset = 0; + } else if length.is_some() { + return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); } if offset != self.expected_next_offset { return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_SEQ); @@ -105,27 +104,41 @@ impl LargeBlobs { )?; client_pin.has_permission(PinPermission::LargeBlobWrite)?; } - if offset + set.len() > self.expected_length { + if offset.saturating_add(set.len()) > self.expected_length { return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); } if offset == 0 { - self.buffer = Vec::with_capacity(self.expected_length); + self.buffer = env.persist().init_large_blob(self.expected_length)?; } - self.buffer.append(&mut set); - self.expected_next_offset = self.buffer.len(); + let received_length = set.len(); + env.persist() + .write_large_blob_chunk(offset, &mut set, &mut self.buffer)?; + self.expected_next_offset += received_length; if self.expected_next_offset == self.expected_length { - self.expected_length = 0; - self.expected_next_offset = 0; - // Must be a positive number. - let buffer_hash_index = self.buffer.len() - TRUNCATED_HASH_LEN; - if Sha::::digest(&self.buffer[..buffer_hash_index])[..TRUNCATED_HASH_LEN] - != self.buffer[buffer_hash_index..] - { - self.buffer = Vec::new(); + const CHUNK_SIZE: usize = 1024; + let mut hash = Sha::::new(); + let buffer_hash_index = self.expected_length.saturating_sub(TRUNCATED_HASH_LEN); + for i in (0..buffer_hash_index).step_by(CHUNK_SIZE) { + let byte_count = cmp::min(buffer_hash_index - i, CHUNK_SIZE); + let chunk = env + .persist() + .get_large_blob(i, byte_count, Some(&self.buffer))? + .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?; + hash.update(&chunk); + } + let mut computed_hash = [0; 32]; + hash.finalize(&mut computed_hash); + let written_hash = env + .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 { return Err(Ctap2StatusCode::CTAP2_ERR_INTEGRITY_FAILURE); } - storage::commit_large_blob_array(env, &self.buffer)?; + env.persist().commit_large_blob_array(&self.buffer)?; self.buffer = Vec::new(); + self.expected_length = 0; + self.expected_next_offset = 0; } return Ok(ResponseData::AuthenticatorLargeBlobs(None)); } @@ -135,6 +148,34 @@ impl LargeBlobs { } } +/// Reads the byte vector stored as the serialized large blobs array. +/// +/// If too few bytes exist at that offset, return the maximum number +/// available. This includes cases of offset being beyond the stored array. +/// +/// If no large blob is committed to the store, get responds as if an empty +/// CBOR array (0x80) was written, together with the 16 byte prefix of its +/// SHA256, to a total length of 17 byte (which is the shortest legitimate +/// large blob entry possible). +fn get_large_blob_array( + env: &mut impl Env, + offset: usize, + 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() + })) +} + #[cfg(test)] mod test { use super::super::data_formats::PinUvAuthProtocol; @@ -144,6 +185,46 @@ mod test { use crate::env::test::TestEnv; use crate::env::EcdhSk; + fn commit_chunk( + env: &mut TestEnv, + large_blobs: &mut LargeBlobState, + data: &[u8], + offset: usize, + length: Option, + ) -> CtapResult { + let key_agreement_key = EcdhSk::::random(env.rng()); + let pin_uv_auth_token = [0x55; 32]; + let mut client_pin = ClientPin::::new_test( + env, + key_agreement_key, + pin_uv_auth_token, + PinUvAuthProtocol::V1, + ); + + let large_blobs_params = AuthenticatorLargeBlobsParameters { + get: None, + set: Some(data.to_vec()), + offset, + length, + pin_uv_auth_param: None, + pin_uv_auth_protocol: None, + }; + large_blobs.process_command(env, &mut client_pin, large_blobs_params) + } + + fn commit_valid_chunk( + env: &mut TestEnv, + large_blobs: &mut LargeBlobState, + data: &[u8], + offset: usize, + length: Option, + ) { + assert_eq!( + commit_chunk(env, large_blobs, data, offset, length), + Ok(ResponseData::AuthenticatorLargeBlobs(None)) + ); + } + #[test] fn test_process_command_get_empty() { let mut env = TestEnv::default(); @@ -155,7 +236,7 @@ mod test { pin_uv_auth_token, PinUvAuthProtocol::V1, ); - let mut large_blobs = LargeBlobs::new(); + let mut large_blobs = LargeBlobState::default(); let large_blob = vec![ 0x80, 0x76, 0xBE, 0x8B, 0x52, 0x8D, 0x00, 0x75, 0xF7, 0xAA, 0xE9, 0x8D, 0x6F, 0xA5, @@ -190,7 +271,7 @@ mod test { pin_uv_auth_token, PinUvAuthProtocol::V1, ); - let mut large_blobs = LargeBlobs::new(); + let mut large_blobs = LargeBlobState::default(); const BLOB_LEN: usize = 200; const DATA_LEN: usize = BLOB_LEN - TRUNCATED_HASH_LEN; @@ -257,7 +338,7 @@ mod test { pin_uv_auth_token, PinUvAuthProtocol::V1, ); - let mut large_blobs = LargeBlobs::new(); + let mut large_blobs = LargeBlobState::default(); const BLOB_LEN: usize = 200; const DATA_LEN: usize = BLOB_LEN - TRUNCATED_HASH_LEN; @@ -308,7 +389,7 @@ mod test { pin_uv_auth_token, PinUvAuthProtocol::V1, ); - let mut large_blobs = LargeBlobs::new(); + let mut large_blobs = LargeBlobState::default(); const BLOB_LEN: usize = 200; const DATA_LEN: usize = BLOB_LEN - TRUNCATED_HASH_LEN; @@ -359,7 +440,7 @@ mod test { pin_uv_auth_token, PinUvAuthProtocol::V1, ); - let mut large_blobs = LargeBlobs::new(); + let mut large_blobs = LargeBlobState::default(); let large_blobs_params = AuthenticatorLargeBlobsParameters { get: Some(1), @@ -386,7 +467,7 @@ mod test { pin_uv_auth_token, PinUvAuthProtocol::V1, ); - let mut large_blobs = LargeBlobs::new(); + let mut large_blobs = LargeBlobState::default(); const BLOB_LEN: usize = 20; // This blob does not have an appropriate hash. @@ -418,7 +499,7 @@ mod test { pin_uv_auth_token, pin_uv_auth_protocol, ); - let mut large_blobs = LargeBlobs::new(); + let mut large_blobs = LargeBlobState::default(); const BLOB_LEN: usize = 20; const DATA_LEN: usize = BLOB_LEN - TRUNCATED_HASH_LEN; @@ -462,4 +543,160 @@ mod test { fn test_process_command_commit_with_pin_v2() { test_helper_process_command_commit_with_pin(PinUvAuthProtocol::V2); } + + #[test] + fn test_commit_get_large_blob_array() { + let mut env = TestEnv::default(); + let mut large_blobs = LargeBlobState::default(); + let mut data = vec![0x01, 0x02, 0x03]; + data.extend_from_slice(&Sha::::digest(&data)[..TRUNCATED_HASH_LEN]); + commit_valid_chunk( + &mut env, + &mut large_blobs, + &data, + 0, + Some(3 + TRUNCATED_HASH_LEN), + ); + + let restored_large_blob_array = get_large_blob_array(&mut env, 0, 1).unwrap(); + assert_eq!(vec![0x01], restored_large_blob_array); + let restored_large_blob_array = get_large_blob_array(&mut env, 1, 1).unwrap(); + assert_eq!(vec![0x02], restored_large_blob_array); + let restored_large_blob_array = get_large_blob_array(&mut env, 2, 1).unwrap(); + assert_eq!(vec![0x03], restored_large_blob_array); + let restored_large_blob_array = + get_large_blob_array(&mut env, 2 + TRUNCATED_HASH_LEN, 2).unwrap(); + assert_eq!(restored_large_blob_array.len(), 1); + let restored_large_blob_array = + get_large_blob_array(&mut env, 3 + TRUNCATED_HASH_LEN, 1).unwrap(); + assert_eq!(Vec::::new(), restored_large_blob_array); + let restored_large_blob_array = + get_large_blob_array(&mut env, 4 + TRUNCATED_HASH_LEN, 1).unwrap(); + assert_eq!(Vec::::new(), restored_large_blob_array); + } + + #[test] + fn test_commit_get_large_blob_array_overwrite() { + let mut env = TestEnv::default(); + let mut large_blobs = LargeBlobState::default(); + + let mut data = vec![0x11; 5]; + data.extend_from_slice(&Sha::::digest(&data)[..TRUNCATED_HASH_LEN]); + commit_valid_chunk( + &mut env, + &mut large_blobs, + &data, + 0, + Some(5 + TRUNCATED_HASH_LEN), + ); + + let mut data = vec![0x22; 4]; + data.extend_from_slice(&Sha::::digest(&data)[..TRUNCATED_HASH_LEN]); + commit_valid_chunk( + &mut env, + &mut large_blobs, + &data, + 0, + Some(4 + TRUNCATED_HASH_LEN), + ); + + let restored_large_blob_array = + get_large_blob_array(&mut env, 0, 4 + TRUNCATED_HASH_LEN).unwrap(); + assert_eq!(data, restored_large_blob_array); + let restored_large_blob_array = + get_large_blob_array(&mut env, 4 + TRUNCATED_HASH_LEN, 1).unwrap(); + assert_eq!(Vec::::new(), restored_large_blob_array); + } + + #[test] + fn test_commit_get_large_blob_array_no_commit() { + let mut env = TestEnv::default(); + + let empty_blob_array = vec![ + 0x80, 0x76, 0xBE, 0x8B, 0x52, 0x8D, 0x00, 0x75, 0xF7, 0xAA, 0xE9, 0x8D, 0x6F, 0xA5, + 0x7A, 0x6D, 0x3C, + ]; + let restored_large_blob_array = get_large_blob_array(&mut env, 0, 17).unwrap(); + assert_eq!(empty_blob_array, restored_large_blob_array); + let restored_large_blob_array = get_large_blob_array(&mut env, 0, 1).unwrap(); + assert_eq!(vec![0x80], restored_large_blob_array); + let restored_large_blob_array = get_large_blob_array(&mut env, 16, 1).unwrap(); + assert_eq!(vec![0x3C], restored_large_blob_array); + } + + #[test] + fn test_commit_large_blob_array_parts() { + let mut env = TestEnv::default(); + let mut large_blobs = LargeBlobState::default(); + + let mut data = vec![0x01, 0x02, 0x03]; + data.extend_from_slice(&Sha::::digest(&data)[..TRUNCATED_HASH_LEN]); + + commit_valid_chunk( + &mut env, + &mut large_blobs, + &data[0..1], + 0, + Some(3 + TRUNCATED_HASH_LEN), + ); + commit_valid_chunk(&mut env, &mut large_blobs, &data[1..2], 1, None); + commit_valid_chunk(&mut env, &mut large_blobs, &data[2..], 2, None); + + let restored_large_blob_array = + get_large_blob_array(&mut env, 0, 3 + TRUNCATED_HASH_LEN).unwrap(); + assert_eq!(data, restored_large_blob_array); + let restored_large_blob_array = + get_large_blob_array(&mut env, 3 + TRUNCATED_HASH_LEN, 1).unwrap(); + assert_eq!(Vec::::new(), restored_large_blob_array); + } + + #[test] + fn test_commit_large_blob_array_invalid() { + let mut env = TestEnv::default(); + let mut large_blobs = LargeBlobState::default(); + + let mut data = vec![0x01, 0x02, 0x03]; + data.extend_from_slice(&Sha::::digest(&data)[..TRUNCATED_HASH_LEN]); + + commit_valid_chunk( + &mut env, + &mut large_blobs, + &data[0..1], + 0, + Some(3 + TRUNCATED_HASH_LEN), + ); + assert_eq!( + commit_chunk(&mut env, &mut large_blobs, &data[1..2], 2, None), + Err(Ctap2StatusCode::CTAP1_ERR_INVALID_SEQ) + ); + + assert_eq!( + commit_chunk( + &mut env, + &mut large_blobs, + &data, + 0, + Some(2 + TRUNCATED_HASH_LEN) + ), + Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER) + ); + + commit_valid_chunk( + &mut env, + &mut large_blobs, + &data[0..2 + TRUNCATED_HASH_LEN], + 0, + Some(3 + TRUNCATED_HASH_LEN), + ); + assert_eq!( + commit_chunk( + &mut env, + &mut large_blobs, + &[data.last().unwrap() ^ 0x01], + 0, + None + ), + Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER) + ); + } } diff --git a/libraries/opensk/src/ctap/mod.rs b/libraries/opensk/src/ctap/mod.rs index 9d223f98..4909333b 100644 --- a/libraries/opensk/src/ctap/mod.rs +++ b/libraries/opensk/src/ctap/mod.rs @@ -51,7 +51,7 @@ use self::data_formats::{ PublicKeyCredentialType, PublicKeyCredentialUserEntity, SignatureAlgorithm, }; use self::hid::{ChannelID, CtapHid, CtapHidCommand, KeepaliveStatus, ProcessedPacket}; -use self::large_blobs::LargeBlobs; +use self::large_blobs::LargeBlobState; use self::response::{ AuthenticatorGetAssertionResponse, AuthenticatorGetInfoResponse, AuthenticatorMakeCredentialResponse, ResponseData, @@ -397,6 +397,7 @@ pub enum StatefulCommand { GetAssertion(Box), EnumerateRps(usize), EnumerateCredentials(Vec), + LargeBlob(LargeBlobState), } /// Stores the current CTAP command state and when it times out. @@ -548,6 +549,24 @@ impl StatefulPermission { Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED) } } + + /// Returns the large blob state. If none is hold, create and hold a new state. + pub fn large_blob(&mut self, env: &mut E, channel: Channel) -> &mut LargeBlobState { + self.clear_timer(env); + if let Some(StatefulCommand::LargeBlob(_)) = &self.command_type { + } else { + self.set_command( + env, + StatefulCommand::LargeBlob(LargeBlobState::default()), + channel, + ); + } + if let Some(StatefulCommand::LargeBlob(ref mut large_blob_state)) = &mut self.command_type { + large_blob_state + } else { + unreachable!(); + } + } } // This struct currently holds all state, not only the persistent memory. The persistent members are @@ -558,7 +577,6 @@ pub struct CtapState { pub(crate) u2f_up_state: U2fUserPresenceState, // The state initializes to Reset and its timeout, and never goes back to Reset. stateful_command_permission: StatefulPermission, - large_blobs: LargeBlobs, } impl CtapState { @@ -575,7 +593,6 @@ impl CtapState { #[cfg(feature = "with_ctap1")] u2f_up_state: U2fUserPresenceState::new(), stateful_command_permission, - large_blobs: LargeBlobs::new(), } } @@ -660,6 +677,7 @@ impl CtapState { self.clear_other_channels(channel); match (&command, self.stateful_command_permission.get_command(env)) { (Command::AuthenticatorGetNextAssertion, Ok(StatefulCommand::GetAssertion(_))) + | (Command::AuthenticatorLargeBlobs(_), Ok(StatefulCommand::LargeBlob(_))) | (Command::AuthenticatorReset, Ok(StatefulCommand::Reset)) // AuthenticatorGetInfo still allows Reset. | (Command::AuthenticatorGetInfo, Ok(StatefulCommand::Reset)) @@ -711,8 +729,8 @@ impl CtapState { ), Command::AuthenticatorSelection => self.process_selection(env, channel), Command::AuthenticatorLargeBlobs(params) => { - self.large_blobs - .process_command(env, &mut self.client_pin, params) + let large_blob_state = self.stateful_command_permission.large_blob(env, channel); + large_blob_state.process_command(env, &mut self.client_pin, params) } #[cfg(feature = "config_command")] Command::AuthenticatorConfig(params) => { @@ -1457,6 +1475,7 @@ mod test { use crate::api::customization; use crate::api::key_store::CBOR_CREDENTIAL_ID_SIZE; use crate::api::user_presence::UserPresenceResult; + use crate::ctap::command::AuthenticatorLargeBlobsParameters; use crate::env::test::TestEnv; use crate::env::EcdhSk; use crate::test_helpers; @@ -3527,4 +3546,91 @@ mod test { Ok(ResponseData::AuthenticatorGetInfo(_)) )); } + + #[test] + fn test_large_blob_stateful() { + let mut env = TestEnv::default(); + let mut ctap_state = CtapState::::new(&mut env); + const EMPTY_BLOB_ARRAY: [u8; 17] = [ + 0x80, 0x76, 0xBE, 0x8B, 0x52, 0x8D, 0x00, 0x75, 0xF7, 0xAA, 0xE9, 0x8D, 0x6F, 0xA5, + 0x7A, 0x6D, 0x3C, + ]; + + let params = AuthenticatorLargeBlobsParameters { + get: None, + set: Some(EMPTY_BLOB_ARRAY[..1].to_vec()), + offset: 0, + length: Some(EMPTY_BLOB_ARRAY.len()), + pin_uv_auth_param: None, + pin_uv_auth_protocol: None, + }; + let response = ctap_state.process_parsed_command( + &mut env, + Command::AuthenticatorLargeBlobs(params), + DUMMY_CHANNEL, + ); + assert_eq!(response, Ok(ResponseData::AuthenticatorLargeBlobs(None))); + + let params = AuthenticatorLargeBlobsParameters { + get: None, + set: Some(EMPTY_BLOB_ARRAY[1..].to_vec()), + offset: 1, + length: None, + pin_uv_auth_param: None, + pin_uv_auth_protocol: None, + }; + let response = ctap_state.process_parsed_command( + &mut env, + Command::AuthenticatorLargeBlobs(params), + DUMMY_CHANNEL, + ); + assert_eq!(response, Ok(ResponseData::AuthenticatorLargeBlobs(None))); + } + + #[test] + fn test_large_blob_stateful_interleaved() { + let mut env = TestEnv::default(); + let mut ctap_state = CtapState::::new(&mut env); + const EMPTY_BLOB_ARRAY: [u8; 17] = [ + 0x80, 0x76, 0xBE, 0x8B, 0x52, 0x8D, 0x00, 0x75, 0xF7, 0xAA, 0xE9, 0x8D, 0x6F, 0xA5, + 0x7A, 0x6D, 0x3C, + ]; + + let params = AuthenticatorLargeBlobsParameters { + get: None, + set: Some(EMPTY_BLOB_ARRAY[..1].to_vec()), + offset: 0, + length: Some(EMPTY_BLOB_ARRAY.len()), + pin_uv_auth_param: None, + pin_uv_auth_protocol: None, + }; + let response = ctap_state.process_parsed_command( + &mut env, + Command::AuthenticatorLargeBlobs(params), + DUMMY_CHANNEL, + ); + assert_eq!(response, Ok(ResponseData::AuthenticatorLargeBlobs(None))); + + let response = ctap_state.process_parsed_command( + &mut env, + Command::AuthenticatorSelection, + DUMMY_CHANNEL, + ); + assert_eq!(response, Ok(ResponseData::AuthenticatorSelection)); + + let params = AuthenticatorLargeBlobsParameters { + get: None, + set: Some(EMPTY_BLOB_ARRAY[1..].to_vec()), + offset: 1, + length: None, + pin_uv_auth_param: None, + pin_uv_auth_protocol: None, + }; + let response = ctap_state.process_parsed_command( + &mut env, + Command::AuthenticatorLargeBlobs(params), + DUMMY_CHANNEL, + ); + assert_eq!(response, Err(Ctap2StatusCode::CTAP1_ERR_INVALID_SEQ)); + } } diff --git a/libraries/opensk/src/ctap/storage.rs b/libraries/opensk/src/ctap/storage.rs index 2754721b..659ad067 100644 --- a/libraries/opensk/src/ctap/storage.rs +++ b/libraries/opensk/src/ctap/storage.rs @@ -22,7 +22,6 @@ use crate::ctap::status_code::Ctap2StatusCode; use crate::env::{AesKey, Env}; use alloc::string::String; use alloc::vec::Vec; -use core::cmp; #[cfg(feature = "config_command")] use sk_cbor::cbor_array_vec; @@ -252,46 +251,6 @@ pub fn set_min_pin_length_rp_ids( .set_min_pin_length_rp_ids(&serialize_min_pin_length_rp_ids(min_pin_length_rp_ids)?) } -/// Reads the byte vector stored as the serialized large blobs array. -/// -/// If too few bytes exist at that offset, return the maximum number -/// available. This includes cases of offset being beyond the stored array. -/// -/// If no large blob is committed to the store, get responds as if an empty -/// CBOR array (0x80) was written, together with the 16 byte prefix of its -/// SHA256, to a total length of 17 byte (which is the shortest legitimate -/// large blob entry possible). -pub fn get_large_blob_array( - env: &mut impl Env, - offset: usize, - byte_count: usize, -) -> Result, Ctap2StatusCode> { - let output = env.persist().get_large_blob_array(offset, byte_count)?; - 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 + byte_count); - EMPTY_LARGE_BLOB - .get(offset..last_index) - .unwrap_or_default() - .to_vec() - })) -} - -/// Sets a byte vector as the serialized large blobs array. -pub fn commit_large_blob_array( - env: &mut impl Env, - large_blob_array: &[u8], -) -> Result<(), Ctap2StatusCode> { - // This input should have been caught at caller level. - if large_blob_array.len() > env.customization().max_large_blob_array_size() { - return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR); - } - env.persist().commit_large_blob_array(large_blob_array) -} - /// Returns whether enterprise attestation is enabled. /// /// Without the AuthenticatorConfig command, customization determines the result. @@ -753,61 +712,6 @@ mod test { assert_eq!(min_pin_length_rp_ids(&mut env).unwrap(), rp_ids); } - #[test] - fn test_commit_get_large_blob_array() { - let mut env = TestEnv::default(); - - let large_blob_array = vec![0x01, 0x02, 0x03]; - assert!(commit_large_blob_array(&mut env, &large_blob_array).is_ok()); - let restored_large_blob_array = get_large_blob_array(&mut env, 0, 1).unwrap(); - assert_eq!(vec![0x01], restored_large_blob_array); - let restored_large_blob_array = get_large_blob_array(&mut env, 1, 1).unwrap(); - assert_eq!(vec![0x02], restored_large_blob_array); - let restored_large_blob_array = get_large_blob_array(&mut env, 2, 1).unwrap(); - assert_eq!(vec![0x03], restored_large_blob_array); - let restored_large_blob_array = get_large_blob_array(&mut env, 2, 2).unwrap(); - assert_eq!(vec![0x03], restored_large_blob_array); - let restored_large_blob_array = get_large_blob_array(&mut env, 3, 1).unwrap(); - assert_eq!(Vec::::new(), restored_large_blob_array); - let restored_large_blob_array = get_large_blob_array(&mut env, 4, 1).unwrap(); - assert_eq!(Vec::::new(), restored_large_blob_array); - } - - #[test] - fn test_commit_get_large_blob_array_overwrite() { - let mut env = TestEnv::default(); - - let large_blob_array = vec![0x11; 5]; - assert!(commit_large_blob_array(&mut env, &large_blob_array).is_ok()); - let large_blob_array = vec![0x22; 4]; - assert!(commit_large_blob_array(&mut env, &large_blob_array).is_ok()); - let restored_large_blob_array = get_large_blob_array(&mut env, 0, 5).unwrap(); - assert_eq!(large_blob_array, restored_large_blob_array); - let restored_large_blob_array = get_large_blob_array(&mut env, 4, 1).unwrap(); - assert_eq!(Vec::::new(), restored_large_blob_array); - - assert!(commit_large_blob_array(&mut env, &[]).is_ok()); - let restored_large_blob_array = get_large_blob_array(&mut env, 0, 20).unwrap(); - // Committing an empty array resets to the default blob of 17 byte. - assert_eq!(restored_large_blob_array.len(), 17); - } - - #[test] - fn test_commit_get_large_blob_array_no_commit() { - let mut env = TestEnv::default(); - - let empty_blob_array = vec![ - 0x80, 0x76, 0xBE, 0x8B, 0x52, 0x8D, 0x00, 0x75, 0xF7, 0xAA, 0xE9, 0x8D, 0x6F, 0xA5, - 0x7A, 0x6D, 0x3C, - ]; - let restored_large_blob_array = get_large_blob_array(&mut env, 0, 17).unwrap(); - assert_eq!(empty_blob_array, restored_large_blob_array); - let restored_large_blob_array = get_large_blob_array(&mut env, 0, 1).unwrap(); - assert_eq!(vec![0x80], restored_large_blob_array); - let restored_large_blob_array = get_large_blob_array(&mut env, 16, 1).unwrap(); - assert_eq!(vec![0x3C], restored_large_blob_array); - } - #[test] fn test_enterprise_attestation() { let mut env = TestEnv::default(); From 3fd77a99c010dc3fb99cde2ab112a97e6b6805c5 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Mon, 8 Apr 2024 14:56:58 +0200 Subject: [PATCH 2/3] 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!(); From 85a06dbd9bedce4600e5eb4c2928759ffcbd712d Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Mon, 8 Apr 2024 16:46:06 +0200 Subject: [PATCH 3/3] Fixes the lifetime --- libraries/opensk/src/api/persist.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/opensk/src/api/persist.rs b/libraries/opensk/src/api/persist.rs index 48d5cd8f..a11548cc 100644 --- a/libraries/opensk/src/api/persist.rs +++ b/libraries/opensk/src/api/persist.rs @@ -265,11 +265,11 @@ pub trait Persist { /// /// The buffer is passed in when writing is in process. fn get_large_blob<'a>( - &'a self, + &self, mut offset: usize, byte_count: usize, buffer: Option<&'a LargeBlobBuffer>, - ) -> CtapResult>> { + ) -> CtapResult>> { if let Some(buffer) = buffer { let start = cmp::min(offset, buffer.len()); let end = offset.saturating_add(byte_count);