diff --git a/libraries/persistent_store/fuzz/Cargo.toml b/libraries/persistent_store/fuzz/Cargo.toml index fdc4f89..f0b01f1 100644 --- a/libraries/persistent_store/fuzz/Cargo.toml +++ b/libraries/persistent_store/fuzz/Cargo.toml @@ -11,6 +11,8 @@ cargo-fuzz = true [dependencies] libfuzzer-sys = "0.3" persistent_store = { path = "..", features = ["std"] } +rand_core = "0.5" +rand_pcg = "0.2" strum = { version = "0.19", features = ["derive"] } # Prevent this from interfering with workspaces diff --git a/libraries/persistent_store/fuzz/fuzz_targets/store.rs b/libraries/persistent_store/fuzz/fuzz_targets/store.rs index 1cff2a4..d96874d 100644 --- a/libraries/persistent_store/fuzz/fuzz_targets/store.rs +++ b/libraries/persistent_store/fuzz/fuzz_targets/store.rs @@ -17,5 +17,5 @@ use libfuzzer_sys::fuzz_target; fuzz_target!(|data: &[u8]| { - // TODO(ia0): Call fuzzing when implemented. + fuzz_store::fuzz(data, false, None); }); diff --git a/libraries/persistent_store/fuzz/src/lib.rs b/libraries/persistent_store/fuzz/src/lib.rs index 11645f3..3d32624 100644 --- a/libraries/persistent_store/fuzz/src/lib.rs +++ b/libraries/persistent_store/fuzz/src/lib.rs @@ -25,13 +25,12 @@ //! situation where coverage takes precedence over surjectivity is for the value of insert updates //! where a pseudo-random generator is used to avoid wasting entropy. -// TODO(ia0): Remove when used. -#![allow(dead_code)] - mod histogram; mod stats; +mod store; pub use stats::{StatKey, Stats}; +pub use store::fuzz; /// Bit-level entropy source based on a byte slice shared reference. /// diff --git a/libraries/persistent_store/fuzz/src/store.rs b/libraries/persistent_store/fuzz/src/store.rs new file mode 100644 index 0000000..a008a4e --- /dev/null +++ b/libraries/persistent_store/fuzz/src/store.rs @@ -0,0 +1,544 @@ +// Copyright 2019-2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use crate::stats::{StatKey, Stats}; +use crate::Entropy; +use persistent_store::{ + BufferOptions, BufferStorage, Store, StoreDriver, StoreDriverOff, StoreDriverOn, + StoreInterruption, StoreInvariant, StoreOperation, StoreUpdate, +}; +use rand_core::{RngCore, SeedableRng}; +use rand_pcg::Pcg32; +use std::collections::HashMap; +use std::convert::TryInto; + +// NOTE: We should be able to improve coverage by only checking the last operation. Because +// operations before the last could be checked with a shorter entropy. + +/// Checks the store against a sequence of manipulations. +/// +/// The entropy to generate the sequence of manipulation should be provided in `data`. Debugging +/// information is printed if `debug` is set. Statistics are gathered if `stats` is set. +pub fn fuzz(data: &[u8], debug: bool, stats: Option<&mut Stats>) { + let mut fuzzer = Fuzzer::new(data, debug, stats); + let mut driver = fuzzer.init(); + let store = loop { + if fuzzer.debug { + print!("{}", driver.storage()); + } + if let StoreDriver::On(driver) = &driver { + if !fuzzer.init.is_dirty() { + driver.check().unwrap(); + } + if fuzzer.debug { + println!("----------------------------------------------------------------------"); + } + } + if fuzzer.entropy.is_empty() { + if fuzzer.debug { + println!("No more entropy."); + } + if fuzzer.init.is_dirty() { + return; + } + fuzzer.record(StatKey::FinishedLifetime, 0); + break driver.power_on().unwrap().extract_store(); + } + driver = match driver { + StoreDriver::On(driver) => match fuzzer.apply(driver) { + Ok(x) => x, + Err(store) => { + if fuzzer.debug { + println!("No more lifetime."); + } + if fuzzer.init.is_dirty() { + return; + } + fuzzer.record(StatKey::FinishedLifetime, 1); + break store; + } + }, + StoreDriver::Off(driver) => fuzzer.power_on(driver), + } + }; + let virt_window = (store.format().num_pages() * store.format().virt_page_size()) as usize; + let init_lifetime = fuzzer.init.used_cycles() * virt_window; + let lifetime = store.lifetime().unwrap().used() - init_lifetime; + fuzzer.record(StatKey::UsedLifetime, lifetime); + fuzzer.record(StatKey::NumCompactions, lifetime / virt_window); + fuzzer.record_counters(); +} + +/// Fuzzing state. +struct Fuzzer<'a> { + /// Remaining fuzzing entropy. + entropy: Entropy<'a>, + + /// Unlimited pseudo entropy. + /// + /// This source is only used to generate the values of entries. This is a compromise to avoid + /// consuming fuzzing entropy for low additional coverage. + values: Pcg32, + + /// The fuzzing mode. + init: Init, + + /// Whether debugging is enabled. + debug: bool, + + /// Whether statistics should be gathered. + stats: Option<&'a mut Stats>, + + /// Statistics counters (only used when gathering statistics). + /// + /// The counters are written to the statistics at the end of the fuzzing run, when their value + /// is final. + counters: HashMap, +} + +impl<'a> Fuzzer<'a> { + /// Creates an initial fuzzing state. + fn new(data: &'a [u8], debug: bool, stats: Option<&'a mut Stats>) -> Fuzzer<'a> { + let mut entropy = Entropy::new(data); + let seed = entropy.read_slice(16); + let values = Pcg32::from_seed(seed[..].try_into().unwrap()); + let mut fuzzer = Fuzzer { + entropy, + values, + init: Init::Clean, + debug, + stats, + counters: HashMap::new(), + }; + fuzzer.init_counters(); + fuzzer.record(StatKey::Entropy, data.len()); + fuzzer + } + + /// Initializes the fuzzing state and returns the store driver. + fn init(&mut self) -> StoreDriver { + let mut options = BufferOptions { + word_size: 4, + page_size: 1 << self.entropy.read_range(5, 12), + max_word_writes: 2, + max_page_erases: self.entropy.read_range(0, 50000), + strict_write: true, + }; + let num_pages = self.entropy.read_range(3, 64); + self.record(StatKey::PageSize, options.page_size); + self.record(StatKey::MaxPageErases, options.max_page_erases); + self.record(StatKey::NumPages, num_pages); + if self.debug { + println!("page_size: {}", options.page_size); + println!("num_pages: {}", num_pages); + println!("max_cycle: {}", options.max_page_erases); + } + let storage_size = num_pages * options.page_size; + if self.entropy.read_bit() { + self.init = Init::Dirty; + let mut storage = vec![0xff; storage_size].into_boxed_slice(); + let length = self.entropy.read_range(0, storage_size); + self.record(StatKey::DirtyLength, length); + for byte in &mut storage[0..length] { + *byte = self.entropy.read_byte(); + } + if self.debug { + println!("Start with dirty storage."); + } + options.strict_write = false; + let storage = BufferStorage::new(storage, options); + StoreDriver::Off(StoreDriverOff::new_dirty(storage)) + } else if self.entropy.read_bit() { + let cycle = self.entropy.read_range(0, options.max_page_erases); + self.init = Init::Used { cycle }; + if self.debug { + println!("Start with {} consumed erase cycles.", cycle); + } + self.record(StatKey::InitCycles, cycle); + let storage = vec![0xff; storage_size].into_boxed_slice(); + let mut storage = BufferStorage::new(storage, options); + Store::init_with_cycle(&mut storage, cycle); + StoreDriver::Off(StoreDriverOff::new_dirty(storage)) + } else { + StoreDriver::Off(StoreDriverOff::new(options, num_pages)) + } + } + + /// Powers a driver with possible interruption. + fn power_on(&mut self, driver: StoreDriverOff) -> StoreDriver { + if self.debug { + println!("Power on the store."); + } + self.increment(StatKey::PowerOnCount); + let interruption = self.interruption(driver.delay_map()); + match driver.partial_power_on(interruption) { + Err((storage, _)) if self.init.is_dirty() => { + self.entropy.consume_all(); + StoreDriver::Off(StoreDriverOff::new_dirty(storage)) + } + Err(error) => self.crash(error), + Ok(driver) => driver, + } + } + + /// Generates and applies an operation with possible interruption. + fn apply(&mut self, driver: StoreDriverOn) -> Result> { + let operation = self.operation(&driver); + if self.debug { + println!("{:?}", operation); + } + let interruption = self.interruption(driver.delay_map(&operation)); + match driver.partial_apply(operation, interruption) { + Err((store, _)) if self.init.is_dirty() => { + self.entropy.consume_all(); + Err(store) + } + Err((store, StoreInvariant::NoLifetime)) => Err(store), + Err((store, error)) => self.crash((store.extract_storage(), error)), + Ok((error, driver)) => { + if self.debug { + if let Some(error) = error { + println!("{:?}", error); + } + } + Ok(driver) + } + } + } + + /// Reports a broken invariant and terminates fuzzing. + fn crash(&self, error: (BufferStorage, StoreInvariant)) -> ! { + let (storage, invariant) = error; + if self.debug { + print!("{}", storage); + } + panic!("{:?}", invariant); + } + + /// Records a statistics if enabled. + fn record(&mut self, key: StatKey, value: usize) { + if let Some(stats) = &mut self.stats { + stats.add(key, value); + } + } + + /// Increments a counter if statistics are enabled. + fn increment(&mut self, key: StatKey) { + if self.stats.is_some() { + *self.counters.get_mut(&key).unwrap() += 1; + } + } + + /// Initializes all counters if statistics are enabled. + fn init_counters(&mut self) { + if self.stats.is_some() { + use StatKey::*; + self.counters.insert(PowerOnCount, 0); + self.counters.insert(TransactionCount, 0); + self.counters.insert(ClearCount, 0); + self.counters.insert(PrepareCount, 0); + self.counters.insert(InsertCount, 0); + self.counters.insert(RemoveCount, 0); + self.counters.insert(InterruptionCount, 0); + } + } + + /// Records all counters if statistics are enabled. + fn record_counters(&mut self) { + if let Some(stats) = &mut self.stats { + for (&key, &value) in self.counters.iter() { + stats.add(key, value); + } + } + } + + /// Generates a possibly invalid operation. + fn operation(&mut self, driver: &StoreDriverOn) -> StoreOperation { + let format = driver.model().format(); + match self.entropy.read_range(0, 2) { + 0 => { + // We also generate an invalid count (one past the maximum value) to test the error + // scenario. Since the test for the error scenario is monotonic, this is a good + // compromise to keep entropy bounded. + let count = self + .entropy + .read_range(0, format.max_updates() as usize + 1); + let mut updates = Vec::with_capacity(count); + for _ in 0..count { + updates.push(self.update()); + } + self.increment(StatKey::TransactionCount); + StoreOperation::Transaction { updates } + } + 1 => { + let min_key = self.key(); + self.increment(StatKey::ClearCount); + StoreOperation::Clear { min_key } + } + 2 => { + // We also generate an invalid length (one past the total capacity) to test the + // error scenario. See the explanation for transactions above for why it's enough. + let length = self + .entropy + .read_range(0, format.total_capacity() as usize + 1); + self.increment(StatKey::PrepareCount); + StoreOperation::Prepare { length } + } + _ => unreachable!(), + } + } + + /// Generates a possibly invalid update. + fn update(&mut self) -> StoreUpdate { + match self.entropy.read_range(0, 1) { + 0 => { + let key = self.key(); + let value = self.value(); + self.increment(StatKey::InsertCount); + StoreUpdate::Insert { key, value } + } + 1 => { + let key = self.key(); + self.increment(StatKey::RemoveCount); + StoreUpdate::Remove { key } + } + _ => unreachable!(), + } + } + + /// Generates a possibly invalid key. + fn key(&mut self) -> usize { + // Use 4096 as the canonical invalid key. + self.entropy.read_range(0, 4096) + } + + /// Generates a possibly invalid value. + fn value(&mut self) -> Vec { + // Use 1024 as the canonical invalid length. + let length = self.entropy.read_range(0, 1024); + let mut value = vec![0; length]; + self.values.fill_bytes(&mut value); + value + } + + /// Generates an interruption. + /// + /// The `delay_map` describes the number of modified bits by the upcoming sequence of store + /// operations. + // TODO(ia0): We use too much CPU to compute the delay map. We should be able to just count the + // number of storage operations by checking the remaining delay. We can then use the entropy + // directly from the corruption function because it's called at most once. + fn interruption( + &mut self, + delay_map: Result, (usize, BufferStorage)>, + ) -> StoreInterruption { + if self.init.is_dirty() { + // We only test that the store can power on without crashing. If it would get + // interrupted then it's like powering up with a different initial state, which would be + // tested with another fuzzing input. + return StoreInterruption::none(); + } + let delay_map = match delay_map { + Ok(x) => x, + Err((delay, storage)) => { + print!("{}", storage); + panic!("delay={}", delay); + } + }; + let delay = self.entropy.read_range(0, delay_map.len() - 1); + let mut complete_bits = BitStack::default(); + for _ in 0..delay_map[delay] { + complete_bits.push(self.entropy.read_bit()); + } + if self.debug { + if delay == delay_map.len() - 1 { + assert!(complete_bits.is_empty()); + println!("Do not interrupt."); + } else { + println!( + "Interrupt after {} operations with complete mask {}.", + delay, complete_bits + ); + } + } + if delay < delay_map.len() - 1 { + self.increment(StatKey::InterruptionCount); + } + let corrupt = Box::new(move |old: &mut [u8], new: &[u8]| { + for (old, new) in old.iter_mut().zip(new.iter()) { + for bit in 0..8 { + let mask = 1 << bit; + if *old & mask == *new & mask { + continue; + } + if complete_bits.pop().unwrap() { + *old ^= mask; + } + } + } + }); + StoreInterruption { delay, corrupt } + } +} + +/// The initial fuzzing mode. +enum Init { + /// Fuzzing starts from a clean storage. + /// + /// All invariants are checked. + Clean, + + /// Fuzzing starts from a dirty storage. + /// + /// Only crashing is checked. + Dirty, + + /// Fuzzing starts from a simulated old storage. + /// + /// All invariants are checked. + Used { + /// Number of simulated used cycles. + cycle: usize, + }, +} + +impl Init { + /// Returns whether fuzzing is in dirty mode. + fn is_dirty(&self) -> bool { + match self { + Init::Dirty => true, + _ => false, + } + } + + /// Returns the number of used cycles. + /// + /// This is zero if the storage was not artificially aged. + fn used_cycles(&self) -> usize { + match self { + Init::Used { cycle } => *cycle, + _ => 0, + } + } +} + +/// Compact stack of bits. +// NOTE: This would probably go away once the delay map is simplified. +#[derive(Default, Clone, Debug)] +struct BitStack { + /// Bits stored in little-endian (for bytes and bits). + /// + /// The last byte only contains `len` bits. + data: Vec, + + /// Number of bits stored in the last byte. + /// + /// It is 0 if the last byte is full, not 8. + len: usize, +} + +impl BitStack { + /// Returns whether the stack is empty. + fn is_empty(&self) -> bool { + self.len() == 0 + } + + /// Returns the length of the stack. + fn len(&self) -> usize { + if self.len == 0 { + 8 * self.data.len() + } else { + 8 * (self.data.len() - 1) + self.len + } + } + + /// Pushes a bit to the stack. + fn push(&mut self, value: bool) { + if self.len == 0 { + self.data.push(0); + } + if value { + *self.data.last_mut().unwrap() |= 1 << self.len; + } + self.len += 1; + if self.len == 8 { + self.len = 0; + } + } + + /// Pops a bit from the stack. + fn pop(&mut self) -> Option { + if self.len == 0 { + if self.data.is_empty() { + return None; + } + self.len = 8; + } + self.len -= 1; + let result = self.data.last().unwrap() & 1 << self.len; + if self.len == 0 { + self.data.pop().unwrap(); + } + Some(result != 0) + } +} + +impl std::fmt::Display for BitStack { + fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { + let mut bits = self.clone(); + while let Some(bit) = bits.pop() { + write!(f, "{}", bit as usize)?; + } + write!(f, " ({} bits)", self.len())?; + Ok(()) + } +} + +#[test] +fn bit_stack_ok() { + let mut bits = BitStack::default(); + + assert_eq!(bits.pop(), None); + + bits.push(true); + assert_eq!(bits.pop(), Some(true)); + assert_eq!(bits.pop(), None); + + bits.push(false); + assert_eq!(bits.pop(), Some(false)); + assert_eq!(bits.pop(), None); + + bits.push(true); + bits.push(false); + assert_eq!(bits.pop(), Some(false)); + assert_eq!(bits.pop(), Some(true)); + assert_eq!(bits.pop(), None); + + bits.push(false); + bits.push(true); + assert_eq!(bits.pop(), Some(true)); + assert_eq!(bits.pop(), Some(false)); + assert_eq!(bits.pop(), None); + + let n = 27; + for i in 0..n { + assert_eq!(bits.len(), i); + bits.push(true); + } + for i in (0..n).rev() { + assert_eq!(bits.pop(), Some(true)); + assert_eq!(bits.len(), i); + } + assert_eq!(bits.pop(), None); +} diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index fa747bc..e7419fd 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -56,7 +56,7 @@ impl TryFrom for PublicKeyCredentialRpEntity { } // https://www.w3.org/TR/webauthn/#dictdef-publickeycredentialuserentity -#[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))] +#[cfg_attr(any(test, feature = "debug_ctap"), derive(Clone, Debug, PartialEq))] pub struct PublicKeyCredentialUserEntity { pub user_id: Vec, pub user_name: Option, @@ -497,10 +497,12 @@ pub struct PublicKeyCredentialSource { pub private_key: ecdsa::SecKey, // TODO(kaczmarczyck) open for other algorithms pub rp_id: String, pub user_handle: Vec, // not optional, but nullable - pub other_ui: Option, + pub user_display_name: Option, pub cred_random: Option>, pub cred_protect_policy: Option, pub creation_order: u64, + pub user_name: Option, + pub user_icon: Option, } // We serialize credentials for the persistent storage using CBOR maps. Each field of a credential @@ -510,10 +512,12 @@ enum PublicKeyCredentialSourceField { PrivateKey = 1, RpId = 2, UserHandle = 3, - OtherUi = 4, + UserDisplayName = 4, CredRandom = 5, CredProtectPolicy = 6, CreationOrder = 7, + UserName = 8, + UserIcon = 9, // When a field is removed, its tag should be reserved and not used for new fields. We document // those reserved tags below. // Reserved tags: none. @@ -534,10 +538,12 @@ impl From for cbor::Value { PublicKeyCredentialSourceField::PrivateKey => Some(private_key.to_vec()), PublicKeyCredentialSourceField::RpId => Some(credential.rp_id), PublicKeyCredentialSourceField::UserHandle => Some(credential.user_handle), - PublicKeyCredentialSourceField::OtherUi => credential.other_ui, + PublicKeyCredentialSourceField::UserDisplayName => credential.user_display_name, PublicKeyCredentialSourceField::CredRandom => credential.cred_random, PublicKeyCredentialSourceField::CredProtectPolicy => credential.cred_protect_policy, PublicKeyCredentialSourceField::CreationOrder => credential.creation_order, + PublicKeyCredentialSourceField::UserName => credential.user_name, + PublicKeyCredentialSourceField::UserIcon => credential.user_icon, } } } @@ -552,10 +558,12 @@ impl TryFrom for PublicKeyCredentialSource { PublicKeyCredentialSourceField::PrivateKey => private_key, PublicKeyCredentialSourceField::RpId => rp_id, PublicKeyCredentialSourceField::UserHandle => user_handle, - PublicKeyCredentialSourceField::OtherUi => other_ui, + PublicKeyCredentialSourceField::UserDisplayName => user_display_name, PublicKeyCredentialSourceField::CredRandom => cred_random, PublicKeyCredentialSourceField::CredProtectPolicy => cred_protect_policy, PublicKeyCredentialSourceField::CreationOrder => creation_order, + PublicKeyCredentialSourceField::UserName => user_name, + PublicKeyCredentialSourceField::UserIcon => user_icon, } = extract_map(cbor_value)?; } @@ -568,12 +576,14 @@ impl TryFrom for PublicKeyCredentialSource { .ok_or(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR)?; let rp_id = extract_text_string(ok_or_missing(rp_id)?)?; let user_handle = extract_byte_string(ok_or_missing(user_handle)?)?; - let other_ui = other_ui.map(extract_text_string).transpose()?; + let user_display_name = user_display_name.map(extract_text_string).transpose()?; let cred_random = cred_random.map(extract_byte_string).transpose()?; let cred_protect_policy = cred_protect_policy .map(CredentialProtectionPolicy::try_from) .transpose()?; let creation_order = creation_order.map(extract_unsigned).unwrap_or(Ok(0))?; + let user_name = user_name.map(extract_text_string).transpose()?; + let user_icon = user_icon.map(extract_text_string).transpose()?; // We don't return whether there were unknown fields in the CBOR value. This means that // deserialization is not injective. In particular deserialization is only an inverse of // serialization at a given version of OpenSK. This is not a problem because: @@ -590,10 +600,12 @@ impl TryFrom for PublicKeyCredentialSource { private_key, rp_id, user_handle, - other_ui, + user_display_name, cred_random, cred_protect_policy, creation_order, + user_name, + user_icon, }) } } @@ -1360,10 +1372,12 @@ mod test { private_key: crypto::ecdsa::SecKey::gensk(&mut rng), rp_id: "example.com".to_string(), user_handle: b"foo".to_vec(), - other_ui: None, + user_display_name: None, cred_random: None, cred_protect_policy: None, creation_order: 0, + user_name: None, + user_icon: None, }; assert_eq!( @@ -1372,7 +1386,7 @@ mod test { ); let credential = PublicKeyCredentialSource { - other_ui: Some("other".to_string()), + user_display_name: Some("Display Name".to_string()), ..credential }; @@ -1396,6 +1410,26 @@ mod test { ..credential }; + assert_eq!( + PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), + Ok(credential.clone()) + ); + + let credential = PublicKeyCredentialSource { + user_name: Some("name".to_string()), + ..credential + }; + + assert_eq!( + PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), + Ok(credential.clone()) + ); + + let credential = PublicKeyCredentialSource { + user_icon: Some("icon".to_string()), + ..credential + }; + assert_eq!( PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), Ok(credential) diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index e3a782a..d9cd11d 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -317,10 +317,12 @@ where private_key: sk, rp_id: String::from(""), user_handle: vec![], - other_ui: None, + user_display_name: None, cred_random, cred_protect_policy: None, creation_order: 0, + user_name: None, + user_icon: None, })) } @@ -534,12 +536,18 @@ where user_handle: user.user_id, // This input is user provided, so we crop it to 64 byte for storage. // The UTF8 encoding is always preserved, so the string might end up shorter. - other_ui: user + user_display_name: user .user_display_name .map(|s| truncate_to_char_boundary(&s, 64).to_string()), cred_random: cred_random.map(|c| c.to_vec()), cred_protect_policy, creation_order: self.persistent_store.new_creation_order()?, + user_name: user + .user_name + .map(|s| truncate_to_char_boundary(&s, 64).to_string()), + user_icon: user + .user_icon + .map(|s| truncate_to_char_boundary(&s, 64).to_string()), }; self.persistent_store.store_credential(credential_source)?; random_id @@ -651,9 +659,9 @@ where let user = if !credential.user_handle.is_empty() { Some(PublicKeyCredentialUserEntity { user_id: credential.user_handle, - user_name: None, - user_display_name: credential.other_ui, - user_icon: None, + user_name: credential.user_name, + user_display_name: credential.user_display_name, + user_icon: credential.user_icon, }) } else { None @@ -772,7 +780,9 @@ where // Remove user identifiable information without uv. if !has_uv { for credential in &mut applicable_credentials { - credential.other_ui = None; + credential.user_name = None; + credential.user_display_name = None; + credential.user_icon = None; } } applicable_credentials.sort_unstable_by_key(|c| c.creation_order); @@ -1169,10 +1179,12 @@ mod test { private_key: excluded_private_key, rp_id: String::from("example.com"), user_handle: vec![], - other_ui: None, + user_display_name: None, cred_random: None, cred_protect_policy: None, creation_order: 0, + user_name: None, + user_icon: None, }; assert!(ctap_state .persistent_store @@ -1357,9 +1369,10 @@ mod test { ); } - fn check_assertion_response( + fn check_assertion_response_with_user( response: Result, - expected_user_id: Vec, + expected_user: PublicKeyCredentialUserEntity, + flags: u8, expected_number_of_credentials: Option, ) { match response.unwrap() { @@ -1373,15 +1386,9 @@ mod test { let expected_auth_data = vec![ 0xA3, 0x79, 0xA6, 0xF6, 0xEE, 0xAF, 0xB9, 0xA5, 0x5E, 0x37, 0x8C, 0x11, 0x80, 0x34, 0xE2, 0x75, 0x1E, 0x68, 0x2F, 0xAB, 0x9F, 0x2D, 0x30, 0xAB, 0x13, 0xD2, - 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, 0x00, 0x00, 0x00, 0x00, 0x01, + 0x12, 0x55, 0x86, 0xCE, 0x19, 0x47, flags, 0x00, 0x00, 0x00, 0x01, ]; assert_eq!(auth_data, expected_auth_data); - let expected_user = PublicKeyCredentialUserEntity { - user_id: expected_user_id, - user_name: None, - user_display_name: None, - user_icon: None, - }; assert_eq!(user, Some(expected_user)); assert_eq!(number_of_credentials, expected_number_of_credentials); } @@ -1389,6 +1396,25 @@ mod test { } } + fn check_assertion_response( + response: Result, + expected_user_id: Vec, + expected_number_of_credentials: Option, + ) { + let expected_user = PublicKeyCredentialUserEntity { + user_id: expected_user_id, + user_name: None, + user_display_name: None, + user_icon: None, + }; + check_assertion_response_with_user( + response, + expected_user, + 0x00, + expected_number_of_credentials, + ); + } + #[test] fn test_residential_process_get_assertion() { let mut rng = ThreadRng256 {}; @@ -1557,12 +1583,14 @@ mod test { private_key: private_key.clone(), rp_id: String::from("example.com"), user_handle: vec![0x1D], - other_ui: None, + user_display_name: None, cred_random: None, cred_protect_policy: Some( CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList, ), creation_order: 0, + user_name: None, + user_icon: None, }; assert!(ctap_state .persistent_store @@ -1616,10 +1644,12 @@ mod test { private_key, rp_id: String::from("example.com"), user_handle: vec![0x1D], - other_ui: None, + user_display_name: None, cred_random: None, cred_protect_policy: Some(CredentialProtectionPolicy::UserVerificationRequired), creation_order: 0, + user_name: None, + user_icon: None, }; assert!(ctap_state .persistent_store @@ -1650,22 +1680,48 @@ mod test { } #[test] - fn test_process_get_next_assertion_two_credentials() { + fn test_process_get_next_assertion_two_credentials_with_uv() { let mut rng = ThreadRng256 {}; + let key_agreement_key = crypto::ecdh::SecKey::gensk(&mut rng); + let pin_uv_auth_token = [0x88; 32]; + let pin_protocol_v1 = PinProtocolV1::new_test(key_agreement_key, pin_uv_auth_token); + let user_immediately_present = |_| Ok(()); let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); + ctap_state.pin_protocol_v1 = pin_protocol_v1; let mut make_credential_params = create_minimal_make_credential_parameters(); - make_credential_params.user.user_id = vec![0x01]; + let user1 = PublicKeyCredentialUserEntity { + user_id: vec![0x01], + user_name: Some("user1".to_string()), + user_display_name: Some("User One".to_string()), + user_icon: Some("icon1".to_string()), + }; + make_credential_params.user = user1.clone(); assert!(ctap_state .process_make_credential(make_credential_params, DUMMY_CHANNEL_ID) .is_ok()); let mut make_credential_params = create_minimal_make_credential_parameters(); - make_credential_params.user.user_id = vec![0x02]; + let user2 = PublicKeyCredentialUserEntity { + user_id: vec![0x02], + user_name: Some("user2".to_string()), + user_display_name: Some("User Two".to_string()), + user_icon: Some("icon2".to_string()), + }; + make_credential_params.user = user2.clone(); assert!(ctap_state .process_make_credential(make_credential_params, DUMMY_CHANNEL_ID) .is_ok()); + ctap_state + .persistent_store + .set_pin_hash(&[0u8; 16]) + .unwrap(); + let pin_uv_auth_param = Some(vec![ + 0x6F, 0x52, 0x83, 0xBF, 0x1A, 0x91, 0xEE, 0x67, 0xE9, 0xD4, 0x4C, 0x80, 0x08, 0x79, + 0x90, 0x8D, + ]); + let get_assertion_params = AuthenticatorGetAssertionParameters { rp_id: String::from("example.com"), client_data_hash: vec![0xCD], @@ -1673,20 +1729,20 @@ mod test { extensions: None, options: GetAssertionOptions { up: false, - uv: false, + uv: true, }, - pin_uv_auth_param: None, - pin_uv_auth_protocol: None, + pin_uv_auth_param, + pin_uv_auth_protocol: Some(1), }; let get_assertion_response = ctap_state.process_get_assertion( get_assertion_params, DUMMY_CHANNEL_ID, DUMMY_CLOCK_VALUE, ); - check_assertion_response(get_assertion_response, vec![0x02], Some(2)); + check_assertion_response_with_user(get_assertion_response, user2, 0x04, Some(2)); let get_assertion_response = ctap_state.process_get_next_assertion(DUMMY_CLOCK_VALUE); - check_assertion_response(get_assertion_response, vec![0x01], None); + check_assertion_response_with_user(get_assertion_response, user1, 0x04, None); let get_assertion_response = ctap_state.process_get_next_assertion(DUMMY_CLOCK_VALUE); assert_eq!( @@ -1696,23 +1752,32 @@ mod test { } #[test] - fn test_process_get_next_assertion_three_credentials() { + fn test_process_get_next_assertion_three_credentials_no_uv() { let mut rng = ThreadRng256 {}; let user_immediately_present = |_| Ok(()); let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE); let mut make_credential_params = create_minimal_make_credential_parameters(); make_credential_params.user.user_id = vec![0x01]; + make_credential_params.user.user_name = Some("removed".to_string()); + make_credential_params.user.user_display_name = Some("removed".to_string()); + make_credential_params.user.user_icon = Some("removed".to_string()); assert!(ctap_state .process_make_credential(make_credential_params, DUMMY_CHANNEL_ID) .is_ok()); let mut make_credential_params = create_minimal_make_credential_parameters(); make_credential_params.user.user_id = vec![0x02]; + make_credential_params.user.user_name = Some("removed".to_string()); + make_credential_params.user.user_display_name = Some("removed".to_string()); + make_credential_params.user.user_icon = Some("removed".to_string()); assert!(ctap_state .process_make_credential(make_credential_params, DUMMY_CHANNEL_ID) .is_ok()); let mut make_credential_params = create_minimal_make_credential_parameters(); make_credential_params.user.user_id = vec![0x03]; + make_credential_params.user.user_name = Some("removed".to_string()); + make_credential_params.user.user_display_name = Some("removed".to_string()); + make_credential_params.user.user_icon = Some("removed".to_string()); assert!(ctap_state .process_make_credential(make_credential_params, DUMMY_CHANNEL_ID) .is_ok()); @@ -1827,10 +1892,12 @@ mod test { private_key, rp_id: String::from("example.com"), user_handle: vec![], - other_ui: None, + user_display_name: None, cred_random: None, cred_protect_policy: None, creation_order: 0, + user_name: None, + user_icon: None, }; assert!(ctap_state .persistent_store diff --git a/src/ctap/pin_protocol_v1.rs b/src/ctap/pin_protocol_v1.rs index 564ca6d..44aad71 100644 --- a/src/ctap/pin_protocol_v1.rs +++ b/src/ctap/pin_protocol_v1.rs @@ -631,6 +631,22 @@ impl PinProtocolV1 { } Ok(()) } + + #[cfg(test)] + pub fn new_test( + key_agreement_key: crypto::ecdh::SecKey, + pin_uv_auth_token: [u8; 32], + ) -> PinProtocolV1 { + PinProtocolV1 { + key_agreement_key, + pin_uv_auth_token, + consecutive_pin_mismatches: 0, + #[cfg(feature = "with_ctap2_1")] + permissions: 0xFF, + #[cfg(feature = "with_ctap2_1")] + permissions_rp_id: None, + } + } } #[cfg(test)] diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index 34ab621..158bc4f 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -646,10 +646,12 @@ mod test { private_key, rp_id: String::from(rp_id), user_handle, - other_ui: None, + user_display_name: None, cred_random: None, cred_protect_policy: None, creation_order: 0, + user_name: None, + user_icon: None, } } @@ -801,12 +803,14 @@ mod test { private_key, rp_id: String::from("example.com"), user_handle: vec![0x00], - other_ui: None, + user_display_name: None, cred_random: None, cred_protect_policy: Some( CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList, ), creation_order: 0, + user_name: None, + user_icon: None, }; assert!(persistent_store.store_credential(credential).is_ok()); @@ -845,10 +849,12 @@ mod test { private_key: key0, rp_id: String::from("example.com"), user_handle: vec![0x00], - other_ui: None, + user_display_name: None, cred_random: None, cred_protect_policy: None, creation_order: 0, + user_name: None, + user_icon: None, }; assert_eq!(found_credential, Some(expected_credential)); } @@ -865,10 +871,12 @@ mod test { private_key, rp_id: String::from("example.com"), user_handle: vec![0x00], - other_ui: None, + user_display_name: None, cred_random: None, cred_protect_policy: Some(CredentialProtectionPolicy::UserVerificationRequired), creation_order: 0, + user_name: None, + user_icon: None, }; assert!(persistent_store.store_credential(credential).is_ok()); @@ -1043,10 +1051,12 @@ mod test { private_key, rp_id: String::from("example.com"), user_handle: vec![0x00], - other_ui: None, + user_display_name: None, cred_random: None, cred_protect_policy: None, creation_order: 0, + user_name: None, + user_icon: None, }; let serialized = serialize_credential(credential.clone()).unwrap(); let reconstructed = deserialize_credential(&serialized).unwrap();