From 11693e50a093f7e9e1bb4e9264d2177d6fe5dc1a Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Wed, 3 Apr 2024 14:31:49 +0200 Subject: [PATCH] Inlining, refactoring and cleanup --- libraries/opensk/src/api/persist.rs | 88 ++++++++++++++- libraries/opensk/src/ctap/client_pin.rs | 28 +++-- libraries/opensk/src/ctap/config_command.rs | 23 ++-- .../opensk/src/ctap/credential_management.rs | 15 +-- libraries/opensk/src/ctap/ctap1.rs | 4 +- libraries/opensk/src/ctap/large_blobs.rs | 5 +- libraries/opensk/src/ctap/mod.rs | 47 ++++---- libraries/opensk/src/ctap/storage.rs | 100 ------------------ libraries/opensk/src/env/mod.rs | 3 - libraries/opensk/src/env/test/mod.rs | 5 - src/env/tock/mod.rs | 5 - 11 files changed, 146 insertions(+), 177 deletions(-) diff --git a/libraries/opensk/src/api/persist.rs b/libraries/opensk/src/api/persist.rs index f5da3228..607b3768 100644 --- a/libraries/opensk/src/api/persist.rs +++ b/libraries/opensk/src/api/persist.rs @@ -32,10 +32,16 @@ pub type PersistCredentialIter<'a> = Box CtapResult>>; /// Inserts the value at the given key. - /// - /// Values up to a length of 1023 Byte must be supported. fn insert(&mut self, key: usize, value: &[u8]) -> CtapResult<()>; /// Removes a key, if present. @@ -55,12 +59,13 @@ pub trait Persist { /// Iterator for all present keys. fn iter(&self) -> CtapResult>; - /// Checks consistency on boot, and if necessary fixes or initializes problems. + /// Checks consistency on boot, and if necessary fixes problems or initializes. + /// + /// Calling this function after successful init should be a NO-OP. fn init(&mut self) -> CtapResult<()> { if self.find(keys::RESET_COMPLETION)?.is_some() { self.reset()?; } - // TODO don't forget to call, add other init functionality Ok(()) } @@ -213,6 +218,8 @@ pub trait Persist { } /// Returns the list of RP IDs that may read the minimum PIN length. + /// + /// Defaults to an empty vector if not found. fn min_pin_length_rp_ids_bytes(&self) -> CtapResult> { Ok(self .find(keys::MIN_PIN_LENGTH_RP_IDS)? @@ -225,6 +232,13 @@ 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. /// Reads the byte vector stored as the serialized large blobs array. /// /// If too few bytes exist at that offset, return the maximum number @@ -352,9 +366,17 @@ pub trait Persist { fn get_attestation(&self, id: AttestationId) -> CtapResult> { let stored_id_bytes = self.find(keys::ATTESTATION_ID)?; if let Some(bytes) = stored_id_bytes { - if bytes.len() != 1 || id != AttestationId::try_from(bytes[0])? { + if bytes.len() != 1 { return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR); } + if id != AttestationId::try_from(bytes[0])? { + return Ok(None); + } + } else { + // This is for backwards compatibility. No ID stored implies batch. + if id != AttestationId::Batch { + return Ok(None); + } } let private_key = self.find(keys::ATTESTATION_PRIVATE_KEY)?; let certificate = self.find(keys::ATTESTATION_CERTIFICATE)?; @@ -446,6 +468,7 @@ pub struct Attestation { mod test { use super::*; use crate::api::customization::Customization; + use crate::api::rng::Rng; use crate::env::test::TestEnv; use crate::env::Env; @@ -469,4 +492,59 @@ mod test { Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR) ); } + + #[test] + fn test_global_signature_counter() { + let mut env = TestEnv::default(); + let persist = env.persist(); + + let mut counter_value = 1; + assert_eq!(persist.global_signature_counter().unwrap(), counter_value); + for increment in 1..10 { + assert!(persist.incr_global_signature_counter(increment).is_ok()); + counter_value += increment; + assert_eq!(persist.global_signature_counter().unwrap(), counter_value); + } + } + + #[test] + fn test_force_pin_change() { + let mut env = TestEnv::default(); + let persist = env.persist(); + + assert!(!persist.has_force_pin_change().unwrap()); + assert_eq!(persist.force_pin_change(), Ok(())); + assert!(persist.has_force_pin_change().unwrap()); + assert_eq!(persist.set_pin(&[0x88; 16], 8), Ok(())); + assert!(!persist.has_force_pin_change().unwrap()); + } + + #[test] + fn test_pin_hash_and_length() { + let mut env = TestEnv::default(); + let random_data = env.rng().gen_uniform_u8x32(); + let persist = env.persist(); + + // Pin hash is initially not set. + assert!(persist.pin_hash().unwrap().is_none()); + assert!(persist.pin_code_point_length().unwrap().is_none()); + + // Setting the pin sets the pin hash. + assert_eq!(random_data.len(), 2 * PIN_AUTH_LENGTH); + let pin_hash_1 = *array_ref!(random_data, 0, PIN_AUTH_LENGTH); + let pin_hash_2 = *array_ref!(random_data, PIN_AUTH_LENGTH, PIN_AUTH_LENGTH); + let pin_length_1 = 4; + let pin_length_2 = 63; + assert_eq!(persist.set_pin(&pin_hash_1, pin_length_1), Ok(())); + assert_eq!(persist.pin_hash().unwrap(), Some(pin_hash_1)); + assert_eq!(persist.pin_code_point_length().unwrap(), Some(pin_length_1)); + assert_eq!(persist.set_pin(&pin_hash_2, pin_length_2), Ok(())); + assert_eq!(persist.pin_hash().unwrap(), Some(pin_hash_2)); + assert_eq!(persist.pin_code_point_length().unwrap(), Some(pin_length_2)); + + // Resetting the storage resets the pin hash. + assert_eq!(persist.reset(), Ok(())); + assert!(persist.pin_hash().unwrap().is_none()); + assert!(persist.pin_code_point_length().unwrap().is_none()); + } } diff --git a/libraries/opensk/src/ctap/client_pin.rs b/libraries/opensk/src/ctap/client_pin.rs index 1a697293..e8bd92e7 100644 --- a/libraries/opensk/src/ctap/client_pin.rs +++ b/libraries/opensk/src/ctap/client_pin.rs @@ -27,6 +27,7 @@ use crate::api::crypto::hmac256::Hmac256; use crate::api::crypto::sha256::Sha256; use crate::api::customization::Customization; use crate::api::key_store::KeyStore; +use crate::api::persist::Persist; use crate::ctap::storage; #[cfg(test)] use crate::env::EcdhSk; @@ -103,11 +104,8 @@ fn check_and_store_new_pin( .key_store() .encrypt_pin_hash(array_ref![pin_hash, 0, PIN_AUTH_LENGTH])?; // The PIN length is always < PIN_PADDED_LENGTH < 256. - storage::set_pin( - env, - array_ref!(pin_hash, 0, PIN_AUTH_LENGTH), - pin_length as u8, - )?; + env.persist() + .set_pin(array_ref!(pin_hash, 0, PIN_AUTH_LENGTH), pin_length as u8)?; Ok(()) } @@ -187,7 +185,7 @@ impl ClientPin { shared_secret: &SharedSecret, pin_hash_enc: Vec, ) -> Result<(), Ctap2StatusCode> { - match storage::pin_hash(env)? { + match env.persist().pin_hash()? { Some(pin_hash) => { if self.consecutive_pin_mismatches >= 3 { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_BLOCKED); @@ -263,7 +261,7 @@ impl ClientPin { let pin_uv_auth_param = ok_or_missing(pin_uv_auth_param)?; let new_pin_enc = ok_or_missing(new_pin_enc)?; - if storage::pin_hash(env)?.is_some() { + if env.persist().pin_hash()?.is_some() { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_AUTH_INVALID); } let shared_secret = self.get_shared_secret(pin_uv_auth_protocol, key_agreement)?; @@ -331,7 +329,7 @@ impl ClientPin { } let shared_secret = self.get_shared_secret(pin_uv_auth_protocol, key_agreement)?; self.verify_pin_hash_enc(env, pin_uv_auth_protocol, &shared_secret, pin_hash_enc)?; - if storage::has_force_pin_change(env)? { + if env.persist().has_force_pin_change()? { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_INVALID); } @@ -618,7 +616,7 @@ mod test { pin[..4].copy_from_slice(b"1234"); let mut pin_hash = [0u8; 16]; pin_hash.copy_from_slice(&Sha::::digest(&pin[..])[..16]); - storage::set_pin(env, &pin_hash, 4).unwrap(); + env.persist().set_pin(&pin_hash, 4).unwrap(); } /// Fails on PINs bigger than 64 bytes. @@ -752,7 +750,7 @@ mod test { 0x01, 0xD9, 0x88, 0x40, 0x50, 0xBB, 0xD0, 0x7A, 0x23, 0x1A, 0xEB, 0x69, 0xD8, 0x36, 0xC4, 0x12, ]; - storage::set_pin(&mut env, &pin_hash, 4).unwrap(); + env.persist().set_pin(&pin_hash, 4).unwrap(); let pin_hash_enc = shared_secret.encrypt(&mut env, &pin_hash).unwrap(); assert_eq!( @@ -1047,7 +1045,7 @@ mod test { let mut env = TestEnv::default(); set_standard_pin(&mut env); - assert_eq!(storage::force_pin_change(&mut env), Ok(())); + assert_eq!(env.persist().force_pin_change(), Ok(())); assert_eq!( client_pin.process_command(&mut env, params), Err(Ctap2StatusCode::CTAP2_ERR_PIN_INVALID), @@ -1157,7 +1155,7 @@ mod test { let mut env = TestEnv::default(); set_standard_pin(&mut env); - assert_eq!(storage::force_pin_change(&mut env), Ok(())); + assert_eq!(env.persist().force_pin_change(), Ok(())); assert_eq!( client_pin.process_command(&mut env, params), Err(Ctap2StatusCode::CTAP2_ERR_PIN_INVALID) @@ -1249,7 +1247,7 @@ mod test { ), ]; for (pin, result) in test_cases { - let old_pin_hash = storage::pin_hash(&mut env).unwrap(); + let old_pin_hash = env.persist().pin_hash().unwrap(); let new_pin_enc = encrypt_pin(&shared_secret, pin); assert_eq!( @@ -1257,9 +1255,9 @@ mod test { result ); if result.is_ok() { - assert_ne!(old_pin_hash, storage::pin_hash(&mut env).unwrap()); + assert_ne!(old_pin_hash, env.persist().pin_hash().unwrap()); } else { - assert_eq!(old_pin_hash, storage::pin_hash(&mut env).unwrap()); + assert_eq!(old_pin_hash, env.persist().pin_hash().unwrap()); } } } diff --git a/libraries/opensk/src/ctap/config_command.rs b/libraries/opensk/src/ctap/config_command.rs index 91371cd9..1f1447d0 100644 --- a/libraries/opensk/src/ctap/config_command.rs +++ b/libraries/opensk/src/ctap/config_command.rs @@ -18,6 +18,7 @@ use super::data_formats::{ConfigSubCommand, ConfigSubCommandParams, SetMinPinLen use super::response::ResponseData; use super::status_code::Ctap2StatusCode; use crate::api::customization::Customization; +use crate::api::persist::Persist; use crate::ctap::storage; use crate::env::Env; use alloc::vec; @@ -56,14 +57,14 @@ fn process_set_min_pin_length( return Err(Ctap2StatusCode::CTAP2_ERR_PIN_POLICY_VIOLATION); } let mut force_change_pin = force_change_pin.unwrap_or(false); - if force_change_pin && storage::pin_hash(env)?.is_none() { + if force_change_pin && env.persist().pin_hash()?.is_none() { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_NOT_SET); } - if let Some(old_length) = storage::pin_code_point_length(env)? { + if let Some(old_length) = env.persist().pin_code_point_length()? { force_change_pin |= new_min_pin_length > old_length; } if force_change_pin { - storage::force_pin_change(env)?; + env.persist().force_pin_change()?; } storage::set_min_pin_length(env, new_min_pin_length)?; if let Some(min_pin_length_rp_ids) = min_pin_length_rp_ids { @@ -87,7 +88,7 @@ pub fn process_config( let enforce_uv = !matches!(sub_command, ConfigSubCommand::ToggleAlwaysUv) && storage::has_always_uv(env)?; - if storage::pin_hash(env)?.is_some() || enforce_uv { + if env.persist().pin_hash()?.is_some() || enforce_uv { let pin_uv_auth_param = pin_uv_auth_param.ok_or(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED)?; let pin_uv_auth_protocol = @@ -211,7 +212,7 @@ mod test { pin_uv_auth_token, pin_uv_auth_protocol, ); - storage::set_pin(&mut env, &[0x88; 16], 4).unwrap(); + env.persist().set_pin(&[0x88; 16], 4).unwrap(); let mut config_data = vec![0xFF; 32]; config_data.extend(&[0x0D, ConfigSubCommand::ToggleAlwaysUv as u8]); @@ -295,7 +296,7 @@ mod test { // Second, increase minimum PIN length from 6 to 8 with PIN auth. // The stored PIN or its length don't matter since we control the token. - storage::set_pin(&mut env, &[0x88; 16], 8).unwrap(); + env.persist().set_pin(&[0x88; 16], 8).unwrap(); let min_pin_length = 8; let mut config_params = create_min_pin_config_params(min_pin_length, None); let pin_uv_auth_param = vec![ @@ -351,7 +352,7 @@ mod test { let min_pin_length = 8; let min_pin_length_rp_ids = vec!["another.example.com".to_string()]; // The stored PIN or its length don't matter since we control the token. - storage::set_pin(&mut env, &[0x88; 16], 8).unwrap(); + env.persist().set_pin(&[0x88; 16], 8).unwrap(); let mut config_params = create_min_pin_config_params(min_pin_length, Some(min_pin_length_rp_ids.clone())); let pin_uv_auth_param = vec![ @@ -414,7 +415,7 @@ mod test { PinUvAuthProtocol::V1, ); - storage::set_pin(&mut env, &[0x88; 16], 4).unwrap(); + env.persist().set_pin(&[0x88; 16], 4).unwrap(); // Increase min PIN, force PIN change. let min_pin_length = 6; let mut config_params = create_min_pin_config_params(min_pin_length, None); @@ -426,7 +427,7 @@ mod test { let config_response = process_config(&mut env, &mut client_pin, config_params); assert_eq!(config_response, Ok(ResponseData::AuthenticatorConfig)); assert_eq!(storage::min_pin_length(&mut env), Ok(min_pin_length)); - assert_eq!(storage::has_force_pin_change(&mut env), Ok(true)); + assert_eq!(env.persist().has_force_pin_change(), Ok(true)); } #[test] @@ -441,7 +442,7 @@ mod test { PinUvAuthProtocol::V1, ); - storage::set_pin(&mut env, &[0x88; 16], 4).unwrap(); + env.persist().set_pin(&[0x88; 16], 4).unwrap(); let pin_uv_auth_param = Some(vec![ 0xE3, 0x74, 0xF4, 0x27, 0xBE, 0x7D, 0x40, 0xB5, 0x71, 0xB6, 0xB4, 0x1A, 0xD2, 0xC1, 0x53, 0xD7, @@ -461,7 +462,7 @@ mod test { }; let config_response = process_config(&mut env, &mut client_pin, config_params); assert_eq!(config_response, Ok(ResponseData::AuthenticatorConfig)); - assert_eq!(storage::has_force_pin_change(&mut env), Ok(true)); + assert_eq!(env.persist().has_force_pin_change(), Ok(true)); } #[test] diff --git a/libraries/opensk/src/ctap/credential_management.rs b/libraries/opensk/src/ctap/credential_management.rs index 6000d39a..11c6767e 100644 --- a/libraries/opensk/src/ctap/credential_management.rs +++ b/libraries/opensk/src/ctap/credential_management.rs @@ -361,6 +361,7 @@ mod test { use super::super::CtapState; use super::*; use crate::api::crypto::ecdh::SecretKey as _; + use crate::api::persist::Persist; use crate::api::private_key::PrivateKey; use crate::api::rng::Rng; use crate::env::test::TestEnv; @@ -401,7 +402,7 @@ mod test { let mut ctap_state = CtapState::new(&mut env); ctap_state.client_pin = client_pin; - storage::set_pin(&mut env, &[0u8; 16], 4).unwrap(); + env.persist().set_pin(&[0u8; 16], 4).unwrap(); let management_data = vec![CredentialManagementSubCommand::GetCredsMetadata as u8]; let pin_uv_auth_param = authenticate_pin_uv_auth_token( &pin_uv_auth_token, @@ -490,7 +491,7 @@ mod test { storage::store_credential(&mut env, credential_source1).unwrap(); storage::store_credential(&mut env, credential_source2).unwrap(); - storage::set_pin(&mut env, &[0u8; 16], 4).unwrap(); + env.persist().set_pin(&[0u8; 16], 4).unwrap(); let pin_uv_auth_param = Some(vec![ 0x1A, 0xA4, 0x96, 0xDA, 0x62, 0x80, 0x28, 0x13, 0xEB, 0x32, 0xB9, 0xF1, 0xD2, 0xA9, 0xD0, 0xD1, @@ -587,7 +588,7 @@ mod test { storage::store_credential(&mut env, credential).unwrap(); } - storage::set_pin(&mut env, &[0u8; 16], 4).unwrap(); + env.persist().set_pin(&[0u8; 16], 4).unwrap(); let pin_uv_auth_param = Some(vec![ 0x1A, 0xA4, 0x96, 0xDA, 0x62, 0x80, 0x28, 0x13, 0xEB, 0x32, 0xB9, 0xF1, 0xD2, 0xA9, 0xD0, 0xD1, @@ -671,7 +672,7 @@ mod test { storage::store_credential(&mut env, credential_source1).unwrap(); storage::store_credential(&mut env, credential_source2).unwrap(); - storage::set_pin(&mut env, &[0u8; 16], 4).unwrap(); + env.persist().set_pin(&[0u8; 16], 4).unwrap(); let pin_uv_auth_param = Some(vec![ 0xF8, 0xB0, 0x3C, 0xC1, 0xD5, 0x58, 0x9C, 0xB7, 0x4D, 0x42, 0xA1, 0x64, 0x14, 0x28, 0x2B, 0x68, @@ -769,7 +770,7 @@ mod test { storage::store_credential(&mut env, credential_source).unwrap(); - storage::set_pin(&mut env, &[0u8; 16], 4).unwrap(); + env.persist().set_pin(&[0u8; 16], 4).unwrap(); let pin_uv_auth_param = Some(vec![ 0xBD, 0xE3, 0xEF, 0x8A, 0x77, 0x01, 0xB1, 0x69, 0x19, 0xE6, 0x62, 0xB9, 0x9B, 0x89, 0x9C, 0x64, @@ -841,7 +842,7 @@ mod test { storage::store_credential(&mut env, credential_source).unwrap(); - storage::set_pin(&mut env, &[0u8; 16], 4).unwrap(); + env.persist().set_pin(&[0u8; 16], 4).unwrap(); let pin_uv_auth_param = Some(vec![ 0xA5, 0x55, 0x8F, 0x03, 0xC3, 0xD3, 0x73, 0x1C, 0x07, 0xDA, 0x1F, 0x8C, 0xC7, 0xBD, 0x9D, 0xB7, @@ -898,7 +899,7 @@ mod test { let mut env = TestEnv::default(); let mut ctap_state = CtapState::new(&mut env); - storage::set_pin(&mut env, &[0u8; 16], 4).unwrap(); + env.persist().set_pin(&[0u8; 16], 4).unwrap(); let cred_management_params = AuthenticatorCredentialManagementParameters { sub_command: CredentialManagementSubCommand::GetCredsMetadata, diff --git a/libraries/opensk/src/ctap/ctap1.rs b/libraries/opensk/src/ctap/ctap1.rs index 5aa7a930..f279f1e1 100644 --- a/libraries/opensk/src/ctap/ctap1.rs +++ b/libraries/opensk/src/ctap/ctap1.rs @@ -641,7 +641,7 @@ mod test { ctap_state.u2f_up_state.grant_up(&mut env); let response = Ctap1Command::process_command(&mut env, &message, &mut ctap_state).unwrap(); assert_eq!(response[0], 0x01); - let global_signature_counter = storage::global_signature_counter(&mut env).unwrap(); + let global_signature_counter = env.persist().global_signature_counter().unwrap(); check_signature_counter( &mut env, array_ref!(response, 1, 4), @@ -666,7 +666,7 @@ mod test { env.clock().advance(TOUCH_TIMEOUT_MS); let response = Ctap1Command::process_command(&mut env, &message, &mut ctap_state).unwrap(); assert_eq!(response[0], 0x01); - let global_signature_counter = storage::global_signature_counter(&mut env).unwrap(); + let global_signature_counter = env.persist().global_signature_counter().unwrap(); check_signature_counter( &mut env, array_ref!(response, 1, 4), diff --git a/libraries/opensk/src/ctap/large_blobs.rs b/libraries/opensk/src/ctap/large_blobs.rs index ff92fa4e..caa0a20a 100644 --- a/libraries/opensk/src/ctap/large_blobs.rs +++ b/libraries/opensk/src/ctap/large_blobs.rs @@ -18,6 +18,7 @@ use super::response::{AuthenticatorLargeBlobsResponse, ResponseData}; use super::status_code::Ctap2StatusCode; use crate::api::crypto::sha256::Sha256; use crate::api::customization::Customization; +use crate::api::persist::Persist; use crate::ctap::storage; use crate::env::{Env, Sha}; use alloc::vec; @@ -86,7 +87,7 @@ impl LargeBlobs { if offset != self.expected_next_offset { return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_SEQ); } - if storage::pin_hash(env)?.is_some() || storage::has_always_uv(env)? { + if env.persist().pin_hash()?.is_some() || storage::has_always_uv(env)? { let pin_uv_auth_param = pin_uv_auth_param.ok_or(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED)?; let pin_uv_auth_protocol = @@ -425,7 +426,7 @@ mod test { large_blob .extend_from_slice(&Sha::::digest(&large_blob[..])[..TRUNCATED_HASH_LEN]); - storage::set_pin(&mut env, &[0u8; 16], 4).unwrap(); + env.persist().set_pin(&[0u8; 16], 4).unwrap(); let mut large_blob_data = vec![0xFF; 32]; // Command constant and offset bytes. large_blob_data.extend(&[0x0C, 0x00, 0x00, 0x00, 0x00, 0x00]); diff --git a/libraries/opensk/src/ctap/mod.rs b/libraries/opensk/src/ctap/mod.rs index 6caa76ca..9d223f98 100644 --- a/libraries/opensk/src/ctap/mod.rs +++ b/libraries/opensk/src/ctap/mod.rs @@ -585,7 +585,7 @@ impl CtapState { ) -> Result<(), Ctap2StatusCode> { if env.customization().use_signature_counter() { let increment = env.rng().next_u32() % 8 + 1; - storage::incr_global_signature_counter(env, increment)?; + env.persist().incr_global_signature_counter(increment)?; } Ok(()) } @@ -744,7 +744,7 @@ impl CtapState { // This case was added in FIDO 2.1. if auth_param.is_empty() { check_user_presence(env, channel)?; - if storage::pin_hash(env)?.is_none() { + if env.persist().pin_hash()?.is_none() { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_NOT_SET); } else { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_INVALID); @@ -811,7 +811,7 @@ impl CtapState { let mut flags = match pin_uv_auth_param { Some(pin_uv_auth_param) => { // This case is not mentioned in CTAP2.1, so we keep 2.0 logic. - if storage::pin_hash(env)?.is_none() { + if env.persist().pin_hash()?.is_none() { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_NOT_SET); } self.client_pin.verify_pin_uv_auth_token( @@ -836,7 +836,7 @@ impl CtapState { return Err(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED); } // Corresponds to makeCredUvNotRqd set to true. - if options.rk && storage::pin_hash(env)?.is_some() { + if options.rk && env.persist().pin_hash()?.is_some() { return Err(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED); } 0x00 @@ -1181,7 +1181,7 @@ impl CtapState { let mut flags = match pin_uv_auth_param { Some(pin_uv_auth_param) => { // This case is not mentioned in CTAP2.1, so we keep 2.0 logic. - if storage::pin_hash(env)?.is_none() { + if env.persist().pin_hash()?.is_none() { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_NOT_SET); } self.client_pin.verify_pin_uv_auth_token( @@ -1318,7 +1318,10 @@ impl CtapState { (String::from("credMgmt"), true), #[cfg(feature = "config_command")] (String::from("authnrCfg"), true), - (String::from("clientPin"), storage::pin_hash(env)?.is_some()), + ( + String::from("clientPin"), + env.persist().pin_hash()?.is_some(), + ), (String::from("largeBlobs"), true), (String::from("pinUvAuthToken"), true), #[cfg(feature = "config_command")] @@ -1358,7 +1361,7 @@ impl CtapState { max_serialized_large_blob_array: Some( env.customization().max_large_blob_array_size() as u64, ), - force_pin_change: Some(storage::has_force_pin_change(env)?), + force_pin_change: Some(env.persist().has_force_pin_change()?), min_pin_length: storage::min_pin_length(env)?, firmware_version: env.firmware_version(), max_cred_blob_length: Some(env.customization().max_cred_blob_length() as u64), @@ -1420,7 +1423,7 @@ impl CtapState { let mut signature_counter = [0u8; 4]; BigEndian::write_u32( &mut signature_counter, - storage::global_signature_counter(env)?, + env.persist().global_signature_counter()?, ); auth_data.extend(&signature_counter); Ok(auth_data) @@ -2050,7 +2053,7 @@ mod test { let mut ctap_state = CtapState::::new(&mut env); ctap_state.client_pin = client_pin; - storage::set_pin(&mut env, &[0x88; 16], 4).unwrap(); + env.persist().set_pin(&[0x88; 16], 4).unwrap(); let client_data_hash = [0xCD]; let pin_uv_auth_param = authenticate_pin_uv_auth_token( @@ -2098,7 +2101,7 @@ mod test { fn test_non_resident_process_make_credential_with_pin() { let mut env = TestEnv::default(); let mut ctap_state = CtapState::::new(&mut env); - storage::set_pin(&mut env, &[0x88; 16], 4).unwrap(); + env.persist().set_pin(&[0x88; 16], 4).unwrap(); let mut make_credential_params = create_minimal_make_credential_parameters(); make_credential_params.options.rk = false; @@ -2118,7 +2121,7 @@ mod test { fn test_resident_process_make_credential_with_pin() { let mut env = TestEnv::default(); let mut ctap_state = CtapState::::new(&mut env); - storage::set_pin(&mut env, &[0x88; 16], 4).unwrap(); + env.persist().set_pin(&[0x88; 16], 4).unwrap(); let make_credential_params = create_minimal_make_credential_parameters(); let make_credential_response = @@ -2143,7 +2146,7 @@ mod test { Err(Ctap2StatusCode::CTAP2_ERR_PUAT_REQUIRED) ); - storage::set_pin(&mut env, &[0x88; 16], 4).unwrap(); + env.persist().set_pin(&[0x88; 16], 4).unwrap(); let mut make_credential_params = create_minimal_make_credential_parameters(); make_credential_params.pin_uv_auth_param = Some(vec![0xA4; 16]); make_credential_params.pin_uv_auth_protocol = Some(PinUvAuthProtocol::V1); @@ -2396,7 +2399,7 @@ mod test { }; let get_assertion_response = ctap_state.process_get_assertion(&mut env, get_assertion_params, DUMMY_CHANNEL); - let signature_counter = storage::global_signature_counter(&mut env).unwrap(); + let signature_counter = env.persist().global_signature_counter().unwrap(); check_assertion_response(get_assertion_response, vec![0x1D], signature_counter, None); } @@ -2621,7 +2624,7 @@ mod test { }; let get_assertion_response = ctap_state.process_get_assertion(&mut env, get_assertion_params, DUMMY_CHANNEL); - let signature_counter = storage::global_signature_counter(&mut env).unwrap(); + let signature_counter = env.persist().global_signature_counter().unwrap(); check_assertion_response(get_assertion_response, vec![0x1D], signature_counter, None); let credential = PublicKeyCredentialSource { @@ -2774,7 +2777,7 @@ mod test { }; let get_assertion_response = ctap_state.process_get_assertion(&mut env, get_assertion_params, DUMMY_CHANNEL); - let signature_counter = storage::global_signature_counter(&mut env).unwrap(); + let signature_counter = env.persist().global_signature_counter().unwrap(); let expected_extension_cbor = [ 0xA1, 0x68, 0x63, 0x72, 0x65, 0x64, 0x42, 0x6C, 0x6F, 0x62, 0x41, 0xCB, ]; @@ -2839,7 +2842,7 @@ mod test { }; let get_assertion_response = ctap_state.process_get_assertion(&mut env, get_assertion_params, DUMMY_CHANNEL); - let signature_counter = storage::global_signature_counter(&mut env).unwrap(); + let signature_counter = env.persist().global_signature_counter().unwrap(); let expected_extension_cbor = [ 0xA1, 0x68, 0x63, 0x72, 0x65, 0x64, 0x42, 0x6C, 0x6F, 0x62, 0x41, 0xCB, ]; @@ -2942,7 +2945,7 @@ mod test { ctap_state.client_pin = client_pin; // The PIN length is outside of the test scope and most likely incorrect. - storage::set_pin(&mut env, &[0u8; 16], 4).unwrap(); + env.persist().set_pin(&[0u8; 16], 4).unwrap(); let client_data_hash = vec![0xCD]; let pin_uv_auth_param = authenticate_pin_uv_auth_token( &pin_uv_auth_token, @@ -2964,7 +2967,7 @@ mod test { }; let get_assertion_response = ctap_state.process_get_assertion(&mut env, get_assertion_params, DUMMY_CHANNEL); - let signature_counter = storage::global_signature_counter(&mut env).unwrap(); + let signature_counter = env.persist().global_signature_counter().unwrap(); check_assertion_response_with_user( get_assertion_response, Some(user2), @@ -3045,7 +3048,7 @@ mod test { }; let get_assertion_response = ctap_state.process_get_assertion(&mut env, get_assertion_params, DUMMY_CHANNEL); - let signature_counter = storage::global_signature_counter(&mut env).unwrap(); + let signature_counter = env.persist().global_signature_counter().unwrap(); check_assertion_response( get_assertion_response, vec![0x03], @@ -3210,13 +3213,13 @@ mod test { let mut env = TestEnv::default(); let mut ctap_state = CtapState::::new(&mut env); - let mut last_counter = storage::global_signature_counter(&mut env).unwrap(); + let mut last_counter = env.persist().global_signature_counter().unwrap(); assert!(last_counter > 0); for _ in 0..100 { assert!(ctap_state .increment_global_signature_counter(&mut env) .is_ok()); - let next_counter = storage::global_signature_counter(&mut env).unwrap(); + let next_counter = env.persist().global_signature_counter().unwrap(); assert!(next_counter > last_counter); last_counter = next_counter; } @@ -3322,7 +3325,7 @@ mod test { storage::store_credential(&mut env, credential).unwrap(); } - storage::set_pin(&mut env, &[0u8; 16], 4).unwrap(); + env.persist().set_pin(&[0u8; 16], 4).unwrap(); let pin_uv_auth_param = Some(vec![ 0x1A, 0xA4, 0x96, 0xDA, 0x62, 0x80, 0x28, 0x13, 0xEB, 0x32, 0xB9, 0xF1, 0xD2, 0xA9, 0xD0, 0xD1, diff --git a/libraries/opensk/src/ctap/storage.rs b/libraries/opensk/src/ctap/storage.rs index 1d078481..2754721b 100644 --- a/libraries/opensk/src/ctap/storage.rs +++ b/libraries/opensk/src/ctap/storage.rs @@ -15,7 +15,6 @@ use crate::api::customization::Customization; use crate::api::key_store::KeyStore; use crate::api::persist::{Persist, PersistCredentialIter}; -use crate::ctap::client_pin::PIN_AUTH_LENGTH; use crate::ctap::data_formats::{ extract_array, extract_text_string, PublicKeyCredentialSource, PublicKeyCredentialUserEntity, }; @@ -190,41 +189,6 @@ pub fn new_creation_order(env: &mut impl Env) -> Result { Ok(max.unwrap_or(0).wrapping_add(1)) } -/// Returns the global signature counter. -pub fn global_signature_counter(env: &mut impl Env) -> Result { - env.persist().global_signature_counter() -} - -/// Increments the global signature counter. -pub fn incr_global_signature_counter( - env: &mut impl Env, - increment: u32, -) -> Result<(), Ctap2StatusCode> { - env.persist().incr_global_signature_counter(increment) -} - -/// Returns the PIN hash if defined. -pub fn pin_hash(env: &mut impl Env) -> Result, Ctap2StatusCode> { - env.persist().pin_hash() -} - -/// Returns the length of the currently set PIN if defined. -#[cfg(feature = "config_command")] -pub fn pin_code_point_length(env: &mut impl Env) -> Result, Ctap2StatusCode> { - env.persist().pin_code_point_length() -} - -/// Sets the PIN hash and length. -/// -/// If it was already defined, it is updated. -pub fn set_pin( - env: &mut impl Env, - pin_hash: &[u8; PIN_AUTH_LENGTH], - pin_code_point_length: u8, -) -> Result<(), Ctap2StatusCode> { - env.persist().set_pin(pin_hash, pin_code_point_length) -} - /// Returns the number of remaining PIN retries. pub fn pin_retries(env: &mut impl Env) -> Result { Ok(env @@ -328,18 +292,6 @@ pub fn commit_large_blob_array( env.persist().commit_large_blob_array(large_blob_array) } -/// Returns whether the PIN needs to be changed before its next usage. -pub fn has_force_pin_change(env: &mut impl Env) -> Result { - // TODO inline some of the single line calls - env.persist().has_force_pin_change() -} - -/// Marks the PIN as outdated with respect to the new PIN policy. -#[cfg(feature = "config_command")] -pub fn force_pin_change(env: &mut impl Env) -> Result<(), Ctap2StatusCode> { - env.persist().force_pin_change() -} - /// Returns whether enterprise attestation is enabled. /// /// Without the AuthenticatorConfig command, customization determines the result. @@ -708,34 +660,6 @@ mod test { assert_eq!(found_credential, Some(expected_credential)); } - #[test] - fn test_pin_hash_and_length() { - let mut env = TestEnv::default(); - - // Pin hash is initially not set. - assert!(pin_hash(&mut env).unwrap().is_none()); - assert!(pin_code_point_length(&mut env).unwrap().is_none()); - - // Setting the pin sets the pin hash. - let random_data = env.rng().gen_uniform_u8x32(); - assert_eq!(random_data.len(), 2 * PIN_AUTH_LENGTH); - let pin_hash_1 = *array_ref!(random_data, 0, PIN_AUTH_LENGTH); - let pin_hash_2 = *array_ref!(random_data, PIN_AUTH_LENGTH, PIN_AUTH_LENGTH); - let pin_length_1 = 4; - let pin_length_2 = 63; - set_pin(&mut env, &pin_hash_1, pin_length_1).unwrap(); - assert_eq!(pin_hash(&mut env).unwrap(), Some(pin_hash_1)); - assert_eq!(pin_code_point_length(&mut env).unwrap(), Some(pin_length_1)); - set_pin(&mut env, &pin_hash_2, pin_length_2).unwrap(); - assert_eq!(pin_hash(&mut env).unwrap(), Some(pin_hash_2)); - assert_eq!(pin_code_point_length(&mut env).unwrap(), Some(pin_length_2)); - - // Resetting the storage resets the pin hash. - reset(&mut env).unwrap(); - assert!(pin_hash(&mut env).unwrap().is_none()); - assert!(pin_code_point_length(&mut env).unwrap().is_none()); - } - #[test] fn test_pin_retries() { let mut env = TestEnv::default(); @@ -884,30 +808,6 @@ mod test { assert_eq!(vec![0x3C], restored_large_blob_array); } - #[test] - fn test_global_signature_counter() { - let mut env = TestEnv::default(); - - let mut counter_value = 1; - assert_eq!(global_signature_counter(&mut env).unwrap(), counter_value); - for increment in 1..10 { - assert!(incr_global_signature_counter(&mut env, increment).is_ok()); - counter_value += increment; - assert_eq!(global_signature_counter(&mut env).unwrap(), counter_value); - } - } - - #[test] - fn test_force_pin_change() { - let mut env = TestEnv::default(); - - assert!(!has_force_pin_change(&mut env).unwrap()); - assert_eq!(force_pin_change(&mut env), Ok(())); - assert!(has_force_pin_change(&mut env).unwrap()); - assert_eq!(set_pin(&mut env, &[0x88; 16], 8), Ok(())); - assert!(!has_force_pin_change(&mut env).unwrap()); - } - #[test] fn test_enterprise_attestation() { let mut env = TestEnv::default(); diff --git a/libraries/opensk/src/env/mod.rs b/libraries/opensk/src/env/mod.rs index ceb6826e..427450fa 100644 --- a/libraries/opensk/src/env/mod.rs +++ b/libraries/opensk/src/env/mod.rs @@ -24,7 +24,6 @@ use crate::api::rng::Rng; use crate::api::user_presence::UserPresence; use crate::ctap::Channel; use alloc::vec::Vec; -use persistent_store::{Storage, Store}; #[cfg(feature = "std")] pub mod test; @@ -44,7 +43,6 @@ pub trait Env { type Rng: Rng; type UserPresence: UserPresence; type Persist: Persist; - type Storage: Storage; type KeyStore: KeyStore; type Write: core::fmt::Write; type Customization: Customization; @@ -55,7 +53,6 @@ pub trait Env { fn rng(&mut self) -> &mut Self::Rng; fn user_presence(&mut self) -> &mut Self::UserPresence; fn persist(&mut self) -> &mut Self::Persist; - fn store(&mut self) -> &mut Store; fn key_store(&mut self) -> &mut Self::KeyStore; fn clock(&mut self) -> &mut Self::Clock; diff --git a/libraries/opensk/src/env/test/mod.rs b/libraries/opensk/src/env/test/mod.rs index ed725580..2d9f6119 100644 --- a/libraries/opensk/src/env/test/mod.rs +++ b/libraries/opensk/src/env/test/mod.rs @@ -189,7 +189,6 @@ impl Env for TestEnv { type Rng = TestRng; type UserPresence = TestUserPresence; type Persist = Self; - type Storage = BufferStorage; type KeyStore = Self; type Clock = TestClock; type Write = TestWrite; @@ -209,10 +208,6 @@ impl Env for TestEnv { self } - fn store(&mut self) -> &mut Store { - &mut self.store - } - fn key_store(&mut self) -> &mut Self { self } diff --git a/src/env/tock/mod.rs b/src/env/tock/mod.rs index 3d1d3932..6ebc5778 100644 --- a/src/env/tock/mod.rs +++ b/src/env/tock/mod.rs @@ -399,7 +399,6 @@ impl E type Rng = TockRng; type UserPresence = Self; type Persist = Self; - type Storage = Storage; type KeyStore = Self; type Clock = TockClock; type Write = ConsoleWriter; @@ -419,10 +418,6 @@ impl E self } - fn store(&mut self) -> &mut Store { - &mut self.store - } - fn key_store(&mut self) -> &mut Self { self }