From d793a992d3abcc68a68bd9aaa97ffa707ebd743c Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Wed, 29 Jun 2022 11:55:02 +0200 Subject: [PATCH 01/10] Add a key store to avoid storing secrets in the store --- src/api/key_store.rs | 138 +++++++++++++++++++++++++++++++++++++ src/api/mod.rs | 1 + src/ctap/crypto_wrapper.rs | 27 +++----- src/ctap/status_code.rs | 7 ++ src/ctap/storage.rs | 57 +-------------- src/ctap/storage/key.rs | 6 +- src/env/mod.rs | 3 + src/env/test/mod.rs | 5 ++ src/env/tock/mod.rs | 5 ++ 9 files changed, 172 insertions(+), 77 deletions(-) create mode 100644 src/api/key_store.rs diff --git a/src/api/key_store.rs b/src/api/key_store.rs new file mode 100644 index 0000000..8d68e58 --- /dev/null +++ b/src/api/key_store.rs @@ -0,0 +1,138 @@ +// Copyright 2022 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 alloc::vec::Vec; +use crypto::ecdsa::SecKey; +use persistent_store::StoreError; +use rng256::Rng256; + +use crate::env::Env; + +/// Provides storage for secret keys. +/// +/// Implementations may use the environment store: [`STORE_KEY`] is reserved for this usage. +pub trait KeyStore { + /// Returns the AES key for key handles encryption. + fn kh_encryption(&mut self) -> Result<[u8; 32], Error>; + + /// Returns the key for key handles authentication. + fn kh_authentication(&mut self) -> Result<[u8; 32], Error>; + + /// Derives an ECDSA private key from a seed. + /// + /// The result is big-endian. + fn derive_ecdsa(&mut self, seed: &[u8; 32]) -> Result<[u8; 32], Error>; + + /// Generates a seed to derive an ECDSA private key. + fn generate_ecdsa_seed(&mut self) -> Result<[u8; 32], Error>; + + /// Resets the key store. + fn reset(&mut self) -> Result<(), Error>; +} + +/// Key store errors. +/// +/// They are deliberately indistinguishable from each other to avoid leaking information. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub struct Error; + +/// Key of the environment store reserved for the key store. +pub const STORE_KEY: usize = 2046; + +impl KeyStore for T { + fn kh_encryption(&mut self) -> Result<[u8; 32], Error> { + Ok(get_master_keys(self)?.encryption) + } + + fn kh_authentication(&mut self) -> Result<[u8; 32], Error> { + Ok(get_master_keys(self)?.authentication) + } + + fn derive_ecdsa(&mut self, seed: &[u8; 32]) -> Result<[u8; 32], Error> { + match SecKey::from_bytes(seed) { + None => Err(Error), + Some(_) => Ok(*seed), + } + } + + fn generate_ecdsa_seed(&mut self) -> Result<[u8; 32], Error> { + let mut seed = [0; 32]; + SecKey::gensk(self.rng()).to_bytes(&mut seed); + Ok(seed) + } + + fn reset(&mut self) -> Result<(), Error> { + Ok(self.store().remove(STORE_KEY)?) + } +} + +/// Wrapper for master keys. +struct MasterKeys { + /// Master encryption key. + encryption: [u8; 32], + + /// Master authentication key. + authentication: [u8; 32], +} + +fn get_master_keys(env: &mut impl Env) -> Result { + let master_keys = match env.store().find(STORE_KEY)? { + Some(x) => x, + None => { + let master_encryption_key = env.rng().gen_uniform_u8x32(); + let master_authentication_key = env.rng().gen_uniform_u8x32(); + let mut master_keys = Vec::with_capacity(64); + master_keys.extend_from_slice(&master_encryption_key); + master_keys.extend_from_slice(&master_authentication_key); + env.store().insert(STORE_KEY, &master_keys)?; + master_keys + } + }; + if master_keys.len() != 64 { + return Err(Error); + } + Ok(MasterKeys { + encryption: *array_ref![master_keys, 0, 32], + authentication: *array_ref![master_keys, 32, 32], + }) +} + +impl From for Error { + fn from(_: StoreError) -> Self { + Error + } +} + +#[test] +fn test_key_store() { + let mut env = crate::env::test::TestEnv::new(); + let key_store = env.key_store(); + + // Master keys are well-defined and stable. + let encryption_key = key_store.kh_encryption().unwrap(); + let authentication_key = key_store.kh_authentication().unwrap(); + assert_eq!(key_store.kh_encryption(), Ok(encryption_key)); + assert_eq!(key_store.kh_authentication(), Ok(authentication_key)); + + // ECDSA seeds are well-defined and stable. + let ecdsa_seed = key_store.generate_ecdsa_seed().unwrap(); + let ecdsa_key = key_store.derive_ecdsa(&ecdsa_seed).unwrap(); + assert_eq!(key_store.derive_ecdsa(&ecdsa_seed), Ok(ecdsa_key)); + + // Master keys change after reset. We don't require this for ECDSA seeds because it's not the + // case, but it might be better. + key_store.reset().unwrap(); + assert!(key_store.kh_encryption().unwrap() != encryption_key); + assert!(key_store.kh_authentication().unwrap() != authentication_key); +} diff --git a/src/api/mod.rs b/src/api/mod.rs index 8e409f6..27f2cdb 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -20,5 +20,6 @@ pub mod connection; pub mod customization; pub mod firmware_protection; +pub mod key_store; pub mod upgrade_storage; pub mod user_presence; diff --git a/src/ctap/crypto_wrapper.rs b/src/ctap/crypto_wrapper.rs index dc0af0c..6f03da1 100644 --- a/src/ctap/crypto_wrapper.rs +++ b/src/ctap/crypto_wrapper.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use crate::api::key_store::KeyStore; #[cfg(feature = "ed25519")] use crate::ctap::data_formats::EDDSA_ALGORITHM; use crate::ctap::data_formats::{ @@ -19,7 +20,6 @@ use crate::ctap::data_formats::{ PublicKeyCredentialType, SignatureAlgorithm, ES256_ALGORITHM, }; use crate::ctap::status_code::Ctap2StatusCode; -use crate::ctap::storage; use crate::env::Env; use alloc::string::String; use alloc::vec; @@ -245,8 +245,7 @@ pub fn encrypt_key_handle( private_key: &PrivateKey, application: &[u8; 32], ) -> Result, Ctap2StatusCode> { - let master_keys = storage::master_keys(env)?; - let aes_enc_key = crypto::aes256::EncryptionKey::new(&master_keys.encryption); + let aes_enc_key = crypto::aes256::EncryptionKey::new(&env.key_store().kh_encryption()?); let mut plaintext = [0; 64]; let version = match private_key { @@ -265,7 +264,7 @@ pub fn encrypt_key_handle( let mut encrypted_id = aes256_cbc_encrypt(env.rng(), &aes_enc_key, &plaintext, true)?; encrypted_id.insert(0, version); - let id_hmac = hmac_256::(&master_keys.hmac, &encrypted_id[..]); + let id_hmac = hmac_256::(&env.key_store().kh_authentication()?, &encrypted_id[..]); encrypted_id.extend(&id_hmac); Ok(encrypted_id) } @@ -289,10 +288,9 @@ pub fn decrypt_credential_source( if credential_id.len() < LEGACY_CREDENTIAL_ID_SIZE { return Ok(None); } - let master_keys = storage::master_keys(env)?; let hmac_message_size = credential_id.len() - 32; if !verify_hmac_256::( - &master_keys.hmac, + &env.key_store().kh_authentication()?, &credential_id[..hmac_message_size], array_ref![credential_id, hmac_message_size, 32], ) { @@ -316,7 +314,7 @@ pub fn decrypt_credential_source( return Ok(None); } - let aes_enc_key = crypto::aes256::EncryptionKey::new(&master_keys.encryption); + let aes_enc_key = crypto::aes256::EncryptionKey::new(&env.key_store().kh_encryption()?); let decrypted_id = aes256_cbc_decrypt(&aes_enc_key, payload, true)?; if rp_id_hash != &decrypted_id[32..] { @@ -548,7 +546,6 @@ mod test { fn test_encrypt_decrypt_credential(signature_algorithm: SignatureAlgorithm) { let mut env = TestEnv::new(); - storage::init(&mut env).ok().unwrap(); let private_key = PrivateKey::new(env.rng(), signature_algorithm); let rp_id_hash = [0x55; 32]; @@ -574,7 +571,6 @@ mod test { #[test] fn test_encrypt_decrypt_bad_version() { let mut env = TestEnv::new(); - storage::init(&mut env).ok().unwrap(); let private_key = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256); let rp_id_hash = [0x55; 32]; @@ -582,8 +578,8 @@ mod test { encrypted_id[0] = UNSUPPORTED_CREDENTIAL_ID_VERSION; // Override the HMAC to pass the check. encrypted_id.truncate(&encrypted_id.len() - 32); - let master_keys = storage::master_keys(&mut env).unwrap(); - let id_hmac = hmac_256::(&master_keys.hmac, &encrypted_id[..]); + let hmac_key = env.key_store().kh_authentication().unwrap(); + let id_hmac = hmac_256::(&hmac_key, &encrypted_id[..]); encrypted_id.extend(&id_hmac); assert_eq!( @@ -594,7 +590,6 @@ mod test { fn test_encrypt_decrypt_bad_hmac(signature_algorithm: SignatureAlgorithm) { let mut env = TestEnv::new(); - storage::init(&mut env).ok().unwrap(); let private_key = PrivateKey::new(env.rng(), signature_algorithm); let rp_id_hash = [0x55; 32]; @@ -622,7 +617,6 @@ mod test { fn test_decrypt_credential_missing_blocks(signature_algorithm: SignatureAlgorithm) { let mut env = TestEnv::new(); - storage::init(&mut env).ok().unwrap(); let private_key = PrivateKey::new(env.rng(), signature_algorithm); let rp_id_hash = [0x55; 32]; @@ -653,14 +647,13 @@ mod test { private_key: crypto::ecdsa::SecKey, application: &[u8; 32], ) -> Result, Ctap2StatusCode> { - let master_keys = storage::master_keys(env)?; - let aes_enc_key = crypto::aes256::EncryptionKey::new(&master_keys.encryption); + let aes_enc_key = crypto::aes256::EncryptionKey::new(&env.key_store().kh_encryption()?); let mut plaintext = [0; 64]; private_key.to_bytes(array_mut_ref!(plaintext, 0, 32)); plaintext[32..64].copy_from_slice(application); let mut encrypted_id = aes256_cbc_encrypt(env.rng(), &aes_enc_key, &plaintext, true)?; - let id_hmac = hmac_256::(&master_keys.hmac, &encrypted_id[..]); + let id_hmac = hmac_256::(&env.key_store().kh_authentication()?, &encrypted_id[..]); encrypted_id.extend(&id_hmac); Ok(encrypted_id) } @@ -668,7 +661,6 @@ mod test { #[test] fn test_encrypt_decrypt_credential_legacy() { let mut env = TestEnv::new(); - storage::init(&mut env).ok().unwrap(); let ecdsa_key = crypto::ecdsa::SecKey::gensk(env.rng()); let private_key = PrivateKey::from(ecdsa_key.clone()); @@ -684,7 +676,6 @@ mod test { #[test] fn test_encrypt_credential_size() { let mut env = TestEnv::new(); - storage::init(&mut env).ok().unwrap(); let private_key = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256); let rp_id_hash = [0x55; 32]; diff --git a/src/ctap/status_code.rs b/src/ctap/status_code.rs index 8d642d5..c6c93c0 100644 --- a/src/ctap/status_code.rs +++ b/src/ctap/status_code.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use crate::api::key_store; use crate::api::user_presence::UserPresenceError; // CTAP specification (version 20190130) section 6.3 @@ -93,3 +94,9 @@ impl From for Ctap2StatusCode { } } } + +impl From for Ctap2StatusCode { + fn from(_: key_store::Error) -> Self { + Self::CTAP2_ERR_VENDOR_INTERNAL_ERROR + } +} diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index d3b3efc..916d5e6 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -15,6 +15,7 @@ mod key; use crate::api::customization::Customization; +use crate::api::key_store::KeyStore; use crate::ctap::client_pin::PIN_AUTH_LENGTH; use crate::ctap::data_formats::{ extract_array, extract_text_string, CredentialProtectionPolicy, PublicKeyCredentialSource, @@ -33,15 +34,6 @@ use persistent_store::{fragment, StoreUpdate}; use rng256::Rng256; use sk_cbor::cbor_array_vec; -/// Wrapper for master keys. -pub struct MasterKeys { - /// Master encryption key. - pub encryption: [u8; 32], - - /// Master hmac key. - pub hmac: [u8; 32], -} - /// Wrapper for PIN properties. struct PinProperties { /// 16 byte prefix of SHA256 of the currently set PIN. @@ -53,16 +45,6 @@ struct PinProperties { /// Initializes the store by creating missing objects. pub fn init(env: &mut impl Env) -> Result<(), Ctap2StatusCode> { - // Generate and store the master keys if they are missing. - if env.store().find_handle(key::MASTER_KEYS)?.is_none() { - let master_encryption_key = env.rng().gen_uniform_u8x32(); - let master_hmac_key = env.rng().gen_uniform_u8x32(); - let mut master_keys = Vec::with_capacity(64); - master_keys.extend_from_slice(&master_encryption_key); - master_keys.extend_from_slice(&master_hmac_key); - env.store().insert(key::MASTER_KEYS, &master_keys)?; - } - // Generate and store the CredRandom secrets if they are missing. if env.store().find_handle(key::CRED_RANDOM_SECRET)?.is_none() { let cred_random_with_uv = env.rng().gen_uniform_u8x32(); @@ -280,21 +262,6 @@ pub fn incr_global_signature_counter( Ok(()) } -/// Returns the master keys. -pub fn master_keys(env: &mut impl Env) -> Result { - let master_keys = env - .store() - .find(key::MASTER_KEYS)? - .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?; - if master_keys.len() != 64 { - return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR); - } - Ok(MasterKeys { - encryption: *array_ref![master_keys, 0, 32], - hmac: *array_ref![master_keys, 32, 32], - }) -} - /// Returns the CredRandom secret. pub fn cred_random_secret(env: &mut impl Env, has_uv: bool) -> Result<[u8; 32], Ctap2StatusCode> { let cred_random_secret = env @@ -549,6 +516,7 @@ pub fn set_aaguid( /// In particular persistent entries are not reset. pub fn reset(env: &mut impl Env) -> Result<(), Ctap2StatusCode> { env.store().clear(key::NUM_PERSISTENT_KEYS)?; + env.key_store().reset()?; init(env)?; Ok(()) } @@ -981,27 +949,6 @@ mod test { assert_eq!(no_credential, None); } - #[test] - fn test_master_keys() { - let mut env = TestEnv::new(); - init(&mut env).unwrap(); - - // Master keys stay the same within the same CTAP reset cycle. - let master_keys_1 = master_keys(&mut env).unwrap(); - let master_keys_2 = master_keys(&mut env).unwrap(); - assert_eq!(master_keys_2.encryption, master_keys_1.encryption); - assert_eq!(master_keys_2.hmac, master_keys_1.hmac); - - // Master keys change after reset. This test may fail if the random generator produces the - // same keys. - let master_encryption_key = master_keys_1.encryption.to_vec(); - let master_hmac_key = master_keys_1.hmac.to_vec(); - reset(&mut env).unwrap(); - let master_keys_3 = master_keys(&mut env).unwrap(); - assert!(master_keys_3.encryption != master_encryption_key.as_slice()); - assert!(master_keys_3.hmac != master_hmac_key.as_slice()); - } - #[test] fn test_cred_random_secret() { let mut env = TestEnv::new(); diff --git a/src/ctap/storage/key.rs b/src/ctap/storage/key.rs index ee9a860..45fef2d 100644 --- a/src/ctap/storage/key.rs +++ b/src/ctap/storage/key.rs @@ -125,10 +125,8 @@ make_partition! { /// the length, the following are an array with the hash. PIN_PROPERTIES = 2045; - /// The encryption and hmac keys. - /// - /// This entry is always present. It is generated at startup if absent. - MASTER_KEYS = 2046; + /// Reserved for the key store implementation of the environment. + _RESERVED_KEY_STORE = 2046; /// The global signature counter. /// diff --git a/src/env/mod.rs b/src/env/mod.rs index 01b7f3a..08bb9f3 100644 --- a/src/env/mod.rs +++ b/src/env/mod.rs @@ -15,6 +15,7 @@ use crate::api::connection::HidConnection; use crate::api::customization::Customization; use crate::api::firmware_protection::FirmwareProtection; +use crate::api::key_store::KeyStore; use crate::api::upgrade_storage::UpgradeStorage; use crate::api::user_presence::UserPresence; use persistent_store::{Storage, Store}; @@ -29,6 +30,7 @@ pub trait Env { type Rng: Rng256; type UserPresence: UserPresence; type Storage: Storage; + type KeyStore: KeyStore; type UpgradeStorage: UpgradeStorage; type FirmwareProtection: FirmwareProtection; type Write: core::fmt::Write; @@ -38,6 +40,7 @@ pub trait Env { fn rng(&mut self) -> &mut Self::Rng; fn user_presence(&mut self) -> &mut Self::UserPresence; fn store(&mut self) -> &mut Store; + fn key_store(&mut self) -> &mut Self::KeyStore; /// Returns the upgrade storage instance. /// diff --git a/src/env/test/mod.rs b/src/env/test/mod.rs index d936577..ea5f2b0 100644 --- a/src/env/test/mod.rs +++ b/src/env/test/mod.rs @@ -151,6 +151,7 @@ impl Env for TestEnv { type Rng = TestRng256; type UserPresence = TestUserPresence; type Storage = BufferStorage; + type KeyStore = Self; type UpgradeStorage = BufferUpgradeStorage; type FirmwareProtection = Self; type Write = TestWrite; @@ -169,6 +170,10 @@ impl Env for TestEnv { &mut self.store } + fn key_store(&mut self) -> &mut Self { + self + } + fn upgrade_storage(&mut self) -> Option<&mut Self::UpgradeStorage> { self.upgrade_storage.as_mut() } diff --git a/src/env/tock/mod.rs b/src/env/tock/mod.rs index 9d124ca..e76088f 100644 --- a/src/env/tock/mod.rs +++ b/src/env/tock/mod.rs @@ -197,6 +197,7 @@ impl Env for TockEnv { type Rng = TockRng256; type UserPresence = Self; type Storage = TockStorage; + type KeyStore = Self; type UpgradeStorage = TockUpgradeStorage; type FirmwareProtection = Self; type Write = Console; @@ -215,6 +216,10 @@ impl Env for TockEnv { &mut self.store } + fn key_store(&mut self) -> &mut Self { + self + } + fn upgrade_storage(&mut self) -> Option<&mut Self::UpgradeStorage> { self.upgrade_storage.as_mut() } From 9a7760f362cea9e3a6bd89c8de9f04c53a500c20 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Thu, 30 Jun 2022 11:21:26 +0200 Subject: [PATCH 02/10] Actually use ECDSA seed mechanism --- src/ctap/credential_management.rs | 33 ++++----- src/ctap/crypto_wrapper.rs | 107 ++++++++++++++++++------------ src/ctap/ctap1.rs | 17 ++--- src/ctap/data_formats.rs | 20 +++--- src/ctap/mod.rs | 44 +++++++----- src/ctap/storage.rs | 48 +++++++------- 6 files changed, 151 insertions(+), 118 deletions(-) diff --git a/src/ctap/credential_management.rs b/src/ctap/credential_management.rs index e4df58c..27fc5a7 100644 --- a/src/ctap/credential_management.rs +++ b/src/ctap/credential_management.rs @@ -64,6 +64,7 @@ fn enumerate_rps_response( /// Generates the response for subcommands enumerating credentials. fn enumerate_credentials_response( + env: &mut impl Env, credential: PublicKeyCredentialSource, total_credentials: Option, ) -> Result { @@ -92,7 +93,9 @@ fn enumerate_credentials_response( key_id: credential_id, transports: None, // You can set USB as a hint here. }; - let public_key = private_key.get_pub_key(); + let public_key = private_key + .get_pub_key(env) + .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?; Ok(AuthenticatorCredentialManagementResponse { user: Some(user), credential_id: Some(credential_id), @@ -207,7 +210,7 @@ fn process_enumerate_credentials_begin( channel, ); } - enumerate_credentials_response(credential, Some(total_credentials as u64)) + enumerate_credentials_response(env, credential, Some(total_credentials as u64)) } /// Processes the subcommand enumerateCredentialsGetNextCredential for CredentialManagement. @@ -217,7 +220,7 @@ fn process_enumerate_credentials_get_next_credential( ) -> Result { let credential_key = stateful_command_permission.next_enumerate_credential()?; let credential = storage::get_credential(env, credential_key)?; - enumerate_credentials_response(credential, None) + enumerate_credentials_response(env, credential, None) } /// Processes the subcommand deleteCredential for CredentialManagement. @@ -369,12 +372,12 @@ mod test { const DUMMY_CHANNEL: Channel = Channel::MainHid([0x12, 0x34, 0x56, 0x78]); - fn create_credential_source(rng: &mut impl Rng256) -> PublicKeyCredentialSource { - let private_key = crypto::ecdsa::SecKey::gensk(rng); + fn create_credential_source(env: &mut TestEnv) -> PublicKeyCredentialSource { + let private_key = PrivateKey::new_ecdsa(env); PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, - credential_id: rng.gen_uniform_u8x32().to_vec(), - private_key: PrivateKey::from(private_key), + credential_id: env.rng().gen_uniform_u8x32().to_vec(), + private_key, rp_id: String::from("example.com"), user_handle: vec![0x01], user_display_name: Some("display_name".to_string()), @@ -393,7 +396,7 @@ mod test { let pin_uv_auth_token = [0x55; 32]; let client_pin = ClientPin::new_test(key_agreement_key, pin_uv_auth_token, pin_uv_auth_protocol); - let credential_source = create_credential_source(env.rng()); + let credential_source = create_credential_source(&mut env); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); ctap_state.client_pin = client_pin; @@ -475,8 +478,8 @@ mod test { let pin_uv_auth_token = [0x55; 32]; let client_pin = ClientPin::new_test(key_agreement_key, pin_uv_auth_token, PinUvAuthProtocol::V1); - let credential_source1 = create_credential_source(env.rng()); - let mut credential_source2 = create_credential_source(env.rng()); + let credential_source1 = create_credential_source(&mut env); + let mut credential_source2 = create_credential_source(&mut env); credential_source2.rp_id = "another.example.com".to_string(); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); @@ -569,7 +572,7 @@ mod test { let pin_uv_auth_token = [0x55; 32]; let client_pin = ClientPin::new_test(key_agreement_key, pin_uv_auth_token, PinUvAuthProtocol::V1); - let credential_source = create_credential_source(env.rng()); + let credential_source = create_credential_source(&mut env); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); ctap_state.client_pin = client_pin; @@ -650,8 +653,8 @@ mod test { let pin_uv_auth_token = [0x55; 32]; let client_pin = ClientPin::new_test(key_agreement_key, pin_uv_auth_token, PinUvAuthProtocol::V1); - let credential_source1 = create_credential_source(env.rng()); - let mut credential_source2 = create_credential_source(env.rng()); + let credential_source1 = create_credential_source(&mut env); + let mut credential_source2 = create_credential_source(&mut env); credential_source2.user_handle = vec![0x02]; credential_source2.user_name = Some("user2".to_string()); credential_source2.user_display_name = Some("User Two".to_string()); @@ -752,7 +755,7 @@ mod test { let pin_uv_auth_token = [0x55; 32]; let client_pin = ClientPin::new_test(key_agreement_key, pin_uv_auth_token, PinUvAuthProtocol::V1); - let mut credential_source = create_credential_source(env.rng()); + let mut credential_source = create_credential_source(&mut env); credential_source.credential_id = vec![0x1D; 32]; let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); @@ -822,7 +825,7 @@ mod test { let pin_uv_auth_token = [0x55; 32]; let client_pin = ClientPin::new_test(key_agreement_key, pin_uv_auth_token, PinUvAuthProtocol::V1); - let mut credential_source = create_credential_source(env.rng()); + let mut credential_source = create_credential_source(&mut env); credential_source.credential_id = vec![0x1D; 32]; let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); diff --git a/src/ctap/crypto_wrapper.rs b/src/ctap/crypto_wrapper.rs index 6f03da1..ea77968 100644 --- a/src/ctap/crypto_wrapper.rs +++ b/src/ctap/crypto_wrapper.rs @@ -101,7 +101,9 @@ pub fn aes256_cbc_decrypt( // We shouldn't compare private keys in prod without constant-time operations. #[cfg_attr(test, derive(PartialEq, Eq))] pub enum PrivateKey { - Ecdsa(ecdsa::SecKey), + // We store the seed instead of the key since we can't get the seed back from the key. We could + // store both if we believe deriving the key is done more than once and costly. + Ecdsa([u8; 32]), #[cfg(feature = "ed25519")] Ed25519(ed25519_compact::SecretKey), } @@ -112,18 +114,25 @@ impl PrivateKey { /// # Panics /// /// Panics if the algorithm is [`SignatureAlgorithm::Unknown`]. - pub fn new(rng: &mut impl Rng256, alg: SignatureAlgorithm) -> Self { + pub fn new(env: &mut impl Env, alg: SignatureAlgorithm) -> Self { match alg { - SignatureAlgorithm::ES256 => PrivateKey::Ecdsa(crypto::ecdsa::SecKey::gensk(rng)), + SignatureAlgorithm::ES256 => { + PrivateKey::Ecdsa(env.key_store().generate_ecdsa_seed().unwrap()) + } #[cfg(feature = "ed25519")] SignatureAlgorithm::EDDSA => { - let bytes = rng.gen_uniform_u8x32(); + let bytes = env.rng().gen_uniform_u8x32(); Self::new_ed25519_from_bytes(&bytes).unwrap() } SignatureAlgorithm::Unknown => unreachable!(), } } + /// Creates a new ecdsa private key. + pub fn new_ecdsa(env: &mut impl Env) -> PrivateKey { + Self::new(env, SignatureAlgorithm::ES256) + } + /// Helper function that creates a private key of type ECDSA. /// /// This function is public for legacy credential source parsing only. @@ -131,7 +140,7 @@ impl PrivateKey { if bytes.len() != 32 { return None; } - ecdsa::SecKey::from_bytes(array_ref!(bytes, 0, 32)).map(PrivateKey::from) + Some(PrivateKey::Ecdsa(*array_ref!(bytes, 0, 32))) } #[cfg(feature = "ed25519")] @@ -143,22 +152,33 @@ impl PrivateKey { Some(Self::Ed25519(ed25519_compact::KeyPair::from_seed(seed).sk)) } - /// Returns the corresponding public key. - pub fn get_pub_key(&self) -> CoseKey { + /// Returns the ECDSA private key. + pub fn ecdsa_key(&self, env: &mut impl Env) -> Option { match self { - PrivateKey::Ecdsa(ecdsa_key) => CoseKey::from(ecdsa_key.genpk()), - #[cfg(feature = "ed25519")] - PrivateKey::Ed25519(ed25519_key) => CoseKey::from(ed25519_key.public_key()), + PrivateKey::Ecdsa(seed) => ecdsa_key(env, seed), + #[allow(unreachable_patterns)] + _ => None, } } + /// Returns the corresponding public key. + pub fn get_pub_key(&self, env: &mut impl Env) -> Option { + Some(match self { + PrivateKey::Ecdsa(ecdsa_seed) => CoseKey::from(ecdsa_key(env, ecdsa_seed)?.genpk()), + #[cfg(feature = "ed25519")] + PrivateKey::Ed25519(ed25519_key) => CoseKey::from(ed25519_key.public_key()), + }) + } + /// Returns the encoded signature for a given message. - pub fn sign_and_encode(&self, message: &[u8]) -> Vec { - match self { - PrivateKey::Ecdsa(ecdsa_key) => ecdsa_key.sign_rfc6979::(message).to_asn1_der(), + pub fn sign_and_encode(&self, env: &mut impl Env, message: &[u8]) -> Option> { + Some(match self { + PrivateKey::Ecdsa(ecdsa_seed) => ecdsa_key(env, ecdsa_seed)? + .sign_rfc6979::(message) + .to_asn1_der(), #[cfg(feature = "ed25519")] PrivateKey::Ed25519(ed25519_key) => ed25519_key.sign(message, None).to_vec(), - } + }) } /// The associated COSE signature algorithm identifier. @@ -173,17 +193,18 @@ impl PrivateKey { /// Writes the key bytes. pub fn to_bytes(&self) -> Vec { match self { - PrivateKey::Ecdsa(ecdsa_key) => { - let mut key_bytes = vec![0u8; 32]; - ecdsa_key.to_bytes(array_mut_ref!(key_bytes, 0, 32)); - key_bytes - } + PrivateKey::Ecdsa(ecdsa_seed) => ecdsa_seed.to_vec(), #[cfg(feature = "ed25519")] PrivateKey::Ed25519(ed25519_key) => ed25519_key.seed().to_vec(), } } } +fn ecdsa_key(env: &mut impl Env, seed: &[u8; 32]) -> Option { + let ecdsa_bytes = env.key_store().derive_ecdsa(seed).ok()?; + ecdsa::SecKey::from_bytes(&ecdsa_bytes) +} + impl From for cbor::Value { fn from(private_key: PrivateKey) -> Self { cbor_array![ @@ -213,12 +234,6 @@ impl TryFrom for PrivateKey { } } -impl From for PrivateKey { - fn from(ecdsa_key: ecdsa::SecKey) -> Self { - PrivateKey::Ecdsa(ecdsa_key) - } -} - /// Encrypts the given private key and relying party ID hash into a credential ID. /// /// Other information, such as a user name, are not stored. Since encrypted credential IDs are @@ -249,8 +264,8 @@ pub fn encrypt_key_handle( let mut plaintext = [0; 64]; let version = match private_key { - PrivateKey::Ecdsa(ecdsa_key) => { - ecdsa_key.to_bytes(array_mut_ref!(plaintext, 0, 32)); + PrivateKey::Ecdsa(ecdsa_seed) => { + plaintext[..32].copy_from_slice(ecdsa_seed); ECDSA_CREDENTIAL_ID_VERSION } #[cfg(feature = "ed25519")] @@ -416,7 +431,7 @@ mod test { #[test] fn test_new_ecdsa_from_bytes() { let mut env = TestEnv::new(); - let private_key = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256); + let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::ES256); let key_bytes = private_key.to_bytes(); assert_eq!( PrivateKey::new_ecdsa_from_bytes(&key_bytes), @@ -456,25 +471,31 @@ mod test { #[test] fn test_private_key_get_pub_key() { let mut env = TestEnv::new(); - let ecdsa_key = crypto::ecdsa::SecKey::gensk(env.rng()); + let private_key = PrivateKey::new_ecdsa(&mut env); + let ecdsa_key = private_key.ecdsa_key(&mut env).unwrap(); let public_key = ecdsa_key.genpk(); - let private_key = PrivateKey::from(ecdsa_key); - assert_eq!(private_key.get_pub_key(), CoseKey::from(public_key)); + assert_eq!( + private_key.get_pub_key(&mut env), + Some(CoseKey::from(public_key)) + ); } #[test] fn test_private_key_sign_and_encode() { let mut env = TestEnv::new(); let message = [0x5A; 32]; - let ecdsa_key = crypto::ecdsa::SecKey::gensk(env.rng()); + let private_key = PrivateKey::new_ecdsa(&mut env); + let ecdsa_key = private_key.ecdsa_key(&mut env).unwrap(); let signature = ecdsa_key.sign_rfc6979::(&message).to_asn1_der(); - let private_key = PrivateKey::from(ecdsa_key); - assert_eq!(private_key.sign_and_encode(&message), signature); + assert_eq!( + private_key.sign_and_encode(&mut env, &message), + Some(signature) + ); } fn test_private_key_signature_algorithm(signature_algorithm: SignatureAlgorithm) { let mut env = TestEnv::new(); - let private_key = PrivateKey::new(env.rng(), signature_algorithm); + let private_key = PrivateKey::new(&mut env, signature_algorithm); assert_eq!(private_key.signature_algorithm(), signature_algorithm); } @@ -491,7 +512,7 @@ mod test { fn test_private_key_from_to_cbor(signature_algorithm: SignatureAlgorithm) { let mut env = TestEnv::new(); - let private_key = PrivateKey::new(env.rng(), signature_algorithm); + let private_key = PrivateKey::new(&mut env, signature_algorithm); let cbor = cbor::Value::from(private_key.clone()); assert_eq!(PrivateKey::try_from(cbor), Ok(private_key),); } @@ -546,7 +567,7 @@ mod test { fn test_encrypt_decrypt_credential(signature_algorithm: SignatureAlgorithm) { let mut env = TestEnv::new(); - let private_key = PrivateKey::new(env.rng(), signature_algorithm); + let private_key = PrivateKey::new(&mut env, signature_algorithm); let rp_id_hash = [0x55; 32]; let encrypted_id = encrypt_key_handle(&mut env, &private_key, &rp_id_hash).unwrap(); @@ -571,7 +592,7 @@ mod test { #[test] fn test_encrypt_decrypt_bad_version() { let mut env = TestEnv::new(); - let private_key = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256); + let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::ES256); let rp_id_hash = [0x55; 32]; let mut encrypted_id = encrypt_key_handle(&mut env, &private_key, &rp_id_hash).unwrap(); @@ -590,7 +611,7 @@ mod test { fn test_encrypt_decrypt_bad_hmac(signature_algorithm: SignatureAlgorithm) { let mut env = TestEnv::new(); - let private_key = PrivateKey::new(env.rng(), signature_algorithm); + let private_key = PrivateKey::new(&mut env, signature_algorithm); let rp_id_hash = [0x55; 32]; let encrypted_id = encrypt_key_handle(&mut env, &private_key, &rp_id_hash).unwrap(); @@ -617,7 +638,7 @@ mod test { fn test_decrypt_credential_missing_blocks(signature_algorithm: SignatureAlgorithm) { let mut env = TestEnv::new(); - let private_key = PrivateKey::new(env.rng(), signature_algorithm); + let private_key = PrivateKey::new(&mut env, signature_algorithm); let rp_id_hash = [0x55; 32]; let encrypted_id = encrypt_key_handle(&mut env, &private_key, &rp_id_hash).unwrap(); @@ -661,8 +682,8 @@ mod test { #[test] fn test_encrypt_decrypt_credential_legacy() { let mut env = TestEnv::new(); - let ecdsa_key = crypto::ecdsa::SecKey::gensk(env.rng()); - let private_key = PrivateKey::from(ecdsa_key.clone()); + let private_key = PrivateKey::new_ecdsa(&mut env); + let ecdsa_key = private_key.ecdsa_key(&mut env).unwrap(); let rp_id_hash = [0x55; 32]; let encrypted_id = legacy_encrypt_key_handle(&mut env, ecdsa_key, &rp_id_hash).unwrap(); @@ -676,7 +697,7 @@ mod test { #[test] fn test_encrypt_credential_size() { let mut env = TestEnv::new(); - let private_key = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256); + let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::ES256); let rp_id_hash = [0x55; 32]; let encrypted_id = encrypt_key_handle(&mut env, &private_key, &rp_id_hash).unwrap(); diff --git a/src/ctap/ctap1.rs b/src/ctap/ctap1.rs index 6dbe55b..2374a2a 100644 --- a/src/ctap/ctap1.rs +++ b/src/ctap/ctap1.rs @@ -245,9 +245,12 @@ impl Ctap1Command { challenge: [u8; 32], application: [u8; 32], ) -> Result, Ctap1StatusCode> { - let sk = crypto::ecdsa::SecKey::gensk(env.rng()); + let private_key = PrivateKey::new_ecdsa(env); + let sk = private_key + .ecdsa_key(env) + .ok_or(Ctap1StatusCode::SW_INTERNAL_EXCEPTION)?; let pk = sk.genpk(); - let key_handle = encrypt_key_handle(env, &PrivateKey::from(sk), &application) + let key_handle = encrypt_key_handle(env, &private_key, &application) .map_err(|_| Ctap1StatusCode::SW_INTERNAL_EXCEPTION)?; if key_handle.len() > 0xFF { // This is just being defensive with unreachable code. @@ -309,12 +312,10 @@ impl Ctap1Command { let credential_source = decrypt_credential_source(env, key_handle, &application) .map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?; if let Some(credential_source) = credential_source { - // CTAP1 only supports ECDSA, the default case applies if CTAP2 adds more algorithms. - #[allow(unreachable_patterns)] - let ecdsa_key = match credential_source.private_key { - PrivateKey::Ecdsa(k) => k, - _ => return Err(Ctap1StatusCode::SW_WRONG_DATA), - }; + let ecdsa_key = credential_source + .private_key + .ecdsa_key(env) + .ok_or(Ctap1StatusCode::SW_WRONG_DATA)?; if flags == Ctap1Flags::CheckOnly { return Err(Ctap1StatusCode::SW_COND_USE_NOT_SATISFIED); } diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index b73f58e..092277e 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -2215,11 +2215,11 @@ mod test { #[test] fn test_credential_source_cbor_round_trip() { let mut env = TestEnv::new(); - let private_key = crypto::ecdsa::SecKey::gensk(env.rng()); + let private_key = PrivateKey::new_ecdsa(&mut env); let credential = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id: env.rng().gen_uniform_u8x32().to_vec(), - private_key: PrivateKey::from(private_key), + private_key, rp_id: "example.com".to_string(), user_handle: b"foo".to_vec(), user_display_name: None, @@ -2300,13 +2300,12 @@ mod test { #[test] fn test_credential_source_cbor_read_legacy() { let mut env = TestEnv::new(); - let private_key = crypto::ecdsa::SecKey::gensk(env.rng()); - let mut key_bytes = [0u8; 32]; - private_key.to_bytes(&mut key_bytes); + let private_key = PrivateKey::new_ecdsa(&mut env); + let key_bytes = private_key.to_bytes(); let credential = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id: env.rng().gen_uniform_u8x32().to_vec(), - private_key: PrivateKey::from(private_key), + private_key, rp_id: "example.com".to_string(), user_handle: b"foo".to_vec(), user_display_name: None, @@ -2333,13 +2332,12 @@ mod test { #[test] fn test_credential_source_cbor_legacy_error() { let mut env = TestEnv::new(); - let private_key = crypto::ecdsa::SecKey::gensk(env.rng()); - let mut key_bytes = [0u8; 32]; - private_key.to_bytes(&mut key_bytes); + let private_key = PrivateKey::new_ecdsa(&mut env); + let key_bytes = private_key.to_bytes(); let credential = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id: env.rng().gen_uniform_u8x32().to_vec(), - private_key: PrivateKey::from(private_key.clone()), + private_key: private_key.clone(), rp_id: "example.com".to_string(), user_handle: b"foo".to_vec(), user_display_name: None, @@ -2366,7 +2364,7 @@ mod test { PublicKeyCredentialSourceField::EcdsaPrivateKey => key_bytes, PublicKeyCredentialSourceField::RpId => credential.rp_id, PublicKeyCredentialSourceField::UserHandle => credential.user_handle, - PublicKeyCredentialSourceField::PrivateKey => PrivateKey::from(private_key), + PublicKeyCredentialSourceField::PrivateKey => private_key, }; assert_eq!( PublicKeyCredentialSource::try_from(source_cbor), diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index f4d1bb7..678cc29 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -853,7 +853,7 @@ impl CtapState { // We decide on the algorithm early, but delay key creation since it takes time. // We rather do that later so all intermediate checks may return faster. - let private_key = PrivateKey::new(env.rng(), algorithm); + let private_key = PrivateKey::new(env, algorithm); let credential_id = if options.rk { let random_id = env.rng().gen_uniform_u8x32().to_vec(); let credential_source = PublicKeyCredentialSource { @@ -892,7 +892,9 @@ impl CtapState { } auth_data.extend(vec![0x00, credential_id.len() as u8]); auth_data.extend(&credential_id); - let public_cose_key = private_key.get_pub_key(); + let public_cose_key = private_key + .get_pub_key(env) + .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?; cbor_write(cbor::Value::from(public_cose_key), &mut auth_data)?; if has_extension_output { let hmac_secret_output = if extensions.hmac_secret { @@ -932,7 +934,12 @@ impl CtapState { Some(vec![attestation_certificate]), ) } else { - (private_key.sign_and_encode(&signature_data), None) + ( + private_key + .sign_and_encode(env, &signature_data) + .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?, + None, + ) }; let attestation_statement = PackedAttestationStatement { alg: SignatureAlgorithm::ES256 as i64, @@ -1014,7 +1021,10 @@ impl CtapState { let mut signature_data = auth_data.clone(); signature_data.extend(client_data_hash); - let signature = credential.private_key.sign_and_encode(&signature_data); + let signature = credential + .private_key + .sign_and_encode(env, &signature_data) + .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?; let cred_desc = PublicKeyCredentialDescriptor { key_type: PublicKeyCredentialType::PublicKey, @@ -1720,7 +1730,7 @@ mod test { #[test] fn test_process_make_credential_credential_excluded() { let mut env = TestEnv::new(); - let excluded_private_key = crypto::ecdsa::SecKey::gensk(env.rng()); + let excluded_private_key = PrivateKey::new_ecdsa(&mut env); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); let excluded_credential_id = vec![0x01, 0x23, 0x45, 0x67]; @@ -1729,7 +1739,7 @@ mod test { let excluded_credential_source = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id: excluded_credential_id, - private_key: PrivateKey::from(excluded_private_key), + private_key: excluded_private_key, rp_id: String::from("example.com"), user_handle: vec![], user_display_name: None, @@ -2538,7 +2548,7 @@ mod test { #[test] fn test_resident_process_get_assertion_with_cred_protect() { let mut env = TestEnv::new(); - let private_key = crypto::ecdsa::SecKey::gensk(env.rng()); + let private_key = PrivateKey::new_ecdsa(&mut env); let credential_id = env.rng().gen_uniform_u8x32().to_vec(); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); @@ -2550,7 +2560,7 @@ mod test { let credential = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id: credential_id.clone(), - private_key: PrivateKey::from(private_key.clone()), + private_key: private_key.clone(), rp_id: String::from("example.com"), user_handle: vec![0x1D], user_display_name: None, @@ -2612,7 +2622,7 @@ mod test { let credential = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id, - private_key: PrivateKey::from(private_key), + private_key, rp_id: String::from("example.com"), user_handle: vec![0x1D], user_display_name: None, @@ -2652,14 +2662,14 @@ mod test { #[test] fn test_process_get_assertion_with_cred_blob() { let mut env = TestEnv::new(); - let private_key = crypto::ecdsa::SecKey::gensk(env.rng()); + let private_key = PrivateKey::new_ecdsa(&mut env); let credential_id = env.rng().gen_uniform_u8x32().to_vec(); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); let credential = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id, - private_key: PrivateKey::from(private_key), + private_key, rp_id: String::from("example.com"), user_handle: vec![0x1D], user_display_name: None, @@ -2710,14 +2720,14 @@ mod test { #[test] fn test_process_get_assertion_with_large_blob_key() { let mut env = TestEnv::new(); - let private_key = crypto::ecdsa::SecKey::gensk(env.rng()); + let private_key = PrivateKey::new_ecdsa(&mut env); let credential_id = env.rng().gen_uniform_u8x32().to_vec(); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); let credential = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id, - private_key: PrivateKey::from(private_key), + private_key, rp_id: String::from("example.com"), user_handle: vec![0x1D], user_display_name: None, @@ -2996,14 +3006,14 @@ mod test { #[test] fn test_process_reset() { let mut env = TestEnv::new(); - let private_key = crypto::ecdsa::SecKey::gensk(env.rng()); + let private_key = PrivateKey::new_ecdsa(&mut env); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); let credential_id = vec![0x01, 0x23, 0x45, 0x67]; let credential_source = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id, - private_key: PrivateKey::from(private_key), + private_key, rp_id: String::from("example.com"), user_handle: vec![], user_display_name: None, @@ -3501,11 +3511,11 @@ mod test { let client_pin = ClientPin::new_test(key_agreement_key, pin_uv_auth_token, PinUvAuthProtocol::V1); - let private_key = crypto::ecdsa::SecKey::gensk(env.rng()); + let private_key = PrivateKey::new_ecdsa(&mut env); let credential_source = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id: env.rng().gen_uniform_u8x32().to_vec(), - private_key: PrivateKey::from(private_key), + private_key, rp_id: String::from("example.com"), user_handle: vec![0x01], user_display_name: Some("display_name".to_string()), diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index 916d5e6..4141003 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -702,15 +702,15 @@ mod test { use rng256::Rng256; fn create_credential_source( - rng: &mut impl Rng256, + env: &mut TestEnv, rp_id: &str, user_handle: Vec, ) -> PublicKeyCredentialSource { - let private_key = crypto::ecdsa::SecKey::gensk(rng); + let private_key = PrivateKey::new_ecdsa(env); PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, - credential_id: rng.gen_uniform_u8x32().to_vec(), - private_key: PrivateKey::from(private_key), + credential_id: env.rng().gen_uniform_u8x32().to_vec(), + private_key, rp_id: String::from(rp_id), user_handle, user_display_name: None, @@ -727,7 +727,7 @@ mod test { fn test_store() { let mut env = TestEnv::new(); assert_eq!(count_credentials(&mut env).unwrap(), 0); - let credential_source = create_credential_source(env.rng(), "example.com", vec![]); + let credential_source = create_credential_source(&mut env, "example.com", vec![]); assert!(store_credential(&mut env, credential_source).is_ok()); assert!(count_credentials(&mut env).unwrap() > 0); } @@ -740,7 +740,7 @@ mod test { let mut credential_ids = vec![]; for i in 0..env.customization().max_supported_resident_keys() { let user_handle = (i as u32).to_ne_bytes().to_vec(); - let credential_source = create_credential_source(env.rng(), "example.com", user_handle); + let credential_source = create_credential_source(&mut env, "example.com", user_handle); credential_ids.push(credential_source.credential_id.clone()); assert!(store_credential(&mut env, credential_source).is_ok()); assert_eq!(count_credentials(&mut env).unwrap(), i + 1); @@ -768,7 +768,7 @@ mod test { Err(Ctap2StatusCode::CTAP2_ERR_NO_CREDENTIALS) ); - let credential_source = create_credential_source(env.rng(), "example.com", vec![0x1D]); + let credential_source = create_credential_source(&mut env, "example.com", vec![0x1D]); let credential_id = credential_source.credential_id.clone(); assert!(store_credential(&mut env, credential_source).is_ok()); let stored_credential = find_credential(&mut env, "example.com", &credential_id, false) @@ -789,10 +789,10 @@ mod test { #[test] fn test_credential_order() { let mut env = TestEnv::new(); - let credential_source = create_credential_source(env.rng(), "example.com", vec![]); + let credential_source = create_credential_source(&mut env, "example.com", vec![]); let current_latest_creation = credential_source.creation_order; assert!(store_credential(&mut env, credential_source).is_ok()); - let mut credential_source = create_credential_source(env.rng(), "example.com", vec![]); + let mut credential_source = create_credential_source(&mut env, "example.com", vec![]); credential_source.creation_order = new_creation_order(&mut env).unwrap(); assert!(credential_source.creation_order > current_latest_creation); let current_latest_creation = credential_source.creation_order; @@ -808,12 +808,12 @@ mod test { let max_supported_resident_keys = env.customization().max_supported_resident_keys(); for i in 0..max_supported_resident_keys { let user_handle = (i as u32).to_ne_bytes().to_vec(); - let credential_source = create_credential_source(env.rng(), "example.com", user_handle); + let credential_source = create_credential_source(&mut env, "example.com", user_handle); assert!(store_credential(&mut env, credential_source).is_ok()); assert_eq!(count_credentials(&mut env).unwrap(), i + 1); } let credential_source = create_credential_source( - env.rng(), + &mut env, "example.com", vec![max_supported_resident_keys as u8], ); @@ -834,8 +834,8 @@ mod test { assert_eq!(count_credentials(&mut env).unwrap(), 0); // These should have different IDs. - let credential_source0 = create_credential_source(env.rng(), "example.com", vec![0x00]); - let credential_source1 = create_credential_source(env.rng(), "example.com", vec![0x00]); + let credential_source0 = create_credential_source(&mut env, "example.com", vec![0x00]); + let credential_source1 = create_credential_source(&mut env, "example.com", vec![0x00]); let credential_id0 = credential_source0.credential_id.clone(); let credential_id1 = credential_source1.credential_id.clone(); @@ -857,12 +857,12 @@ mod test { let max_supported_resident_keys = env.customization().max_supported_resident_keys(); for i in 0..max_supported_resident_keys { let user_handle = (i as u32).to_ne_bytes().to_vec(); - let credential_source = create_credential_source(env.rng(), "example.com", user_handle); + let credential_source = create_credential_source(&mut env, "example.com", user_handle); assert!(store_credential(&mut env, credential_source).is_ok()); assert_eq!(count_credentials(&mut env).unwrap(), i + 1); } let credential_source = create_credential_source( - env.rng(), + &mut env, "example.com", vec![max_supported_resident_keys as u8], ); @@ -879,10 +879,10 @@ mod test { #[test] fn test_get_credential() { let mut env = TestEnv::new(); - let credential_source0 = create_credential_source(env.rng(), "example.com", vec![0x00]); - let credential_source1 = create_credential_source(env.rng(), "example.com", vec![0x01]); + let credential_source0 = create_credential_source(&mut env, "example.com", vec![0x00]); + let credential_source1 = create_credential_source(&mut env, "example.com", vec![0x01]); let credential_source2 = - create_credential_source(env.rng(), "another.example.com", vec![0x02]); + create_credential_source(&mut env, "another.example.com", vec![0x02]); let credential_sources = vec![credential_source0, credential_source1, credential_source2]; for credential_source in credential_sources.into_iter() { let cred_id = credential_source.credential_id.clone(); @@ -897,8 +897,8 @@ mod test { fn test_find() { let mut env = TestEnv::new(); assert_eq!(count_credentials(&mut env).unwrap(), 0); - let credential_source0 = create_credential_source(env.rng(), "example.com", vec![0x00]); - let credential_source1 = create_credential_source(env.rng(), "example.com", vec![0x01]); + let credential_source0 = create_credential_source(&mut env, "example.com", vec![0x00]); + let credential_source1 = create_credential_source(&mut env, "example.com", vec![0x01]); let id0 = credential_source0.credential_id.clone(); let key0 = credential_source0.private_key.clone(); assert!(store_credential(&mut env, credential_source0).is_ok()); @@ -928,11 +928,11 @@ mod test { fn test_find_with_cred_protect() { let mut env = TestEnv::new(); assert_eq!(count_credentials(&mut env).unwrap(), 0); - let private_key = crypto::ecdsa::SecKey::gensk(env.rng()); + let private_key = PrivateKey::new_ecdsa(&mut env); let credential = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id: env.rng().gen_uniform_u8x32().to_vec(), - private_key: PrivateKey::from(private_key), + private_key, rp_id: String::from("example.com"), user_handle: vec![0x00], user_display_name: None, @@ -1228,11 +1228,11 @@ mod test { #[test] fn test_serialize_deserialize_credential() { let mut env = TestEnv::new(); - let private_key = crypto::ecdsa::SecKey::gensk(env.rng()); + let private_key = PrivateKey::new_ecdsa(&mut env); let credential = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id: env.rng().gen_uniform_u8x32().to_vec(), - private_key: PrivateKey::from(private_key), + private_key, rp_id: String::from("example.com"), user_handle: vec![0x00], user_display_name: Some(String::from("Display Name")), From fcdf617a2e60ab7d250468c05f8ab42a908f5638 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Thu, 30 Jun 2022 11:38:03 +0200 Subject: [PATCH 03/10] Rename kh_ to key_handle_ --- src/api/key_store.rs | 23 +++++++++++++---------- src/ctap/crypto_wrapper.rs | 21 ++++++++++++++------- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/api/key_store.rs b/src/api/key_store.rs index 8d68e58..5bbfae2 100644 --- a/src/api/key_store.rs +++ b/src/api/key_store.rs @@ -24,10 +24,10 @@ use crate::env::Env; /// Implementations may use the environment store: [`STORE_KEY`] is reserved for this usage. pub trait KeyStore { /// Returns the AES key for key handles encryption. - fn kh_encryption(&mut self) -> Result<[u8; 32], Error>; + fn key_handle_encryption(&mut self) -> Result<[u8; 32], Error>; /// Returns the key for key handles authentication. - fn kh_authentication(&mut self) -> Result<[u8; 32], Error>; + fn key_handle_authentication(&mut self) -> Result<[u8; 32], Error>; /// Derives an ECDSA private key from a seed. /// @@ -51,11 +51,11 @@ pub struct Error; pub const STORE_KEY: usize = 2046; impl KeyStore for T { - fn kh_encryption(&mut self) -> Result<[u8; 32], Error> { + fn key_handle_encryption(&mut self) -> Result<[u8; 32], Error> { Ok(get_master_keys(self)?.encryption) } - fn kh_authentication(&mut self) -> Result<[u8; 32], Error> { + fn key_handle_authentication(&mut self) -> Result<[u8; 32], Error> { Ok(get_master_keys(self)?.authentication) } @@ -120,10 +120,13 @@ fn test_key_store() { let key_store = env.key_store(); // Master keys are well-defined and stable. - let encryption_key = key_store.kh_encryption().unwrap(); - let authentication_key = key_store.kh_authentication().unwrap(); - assert_eq!(key_store.kh_encryption(), Ok(encryption_key)); - assert_eq!(key_store.kh_authentication(), Ok(authentication_key)); + let encryption_key = key_store.key_handle_encryption().unwrap(); + let authentication_key = key_store.key_handle_authentication().unwrap(); + assert_eq!(key_store.key_handle_encryption(), Ok(encryption_key)); + assert_eq!( + key_store.key_handle_authentication(), + Ok(authentication_key) + ); // ECDSA seeds are well-defined and stable. let ecdsa_seed = key_store.generate_ecdsa_seed().unwrap(); @@ -133,6 +136,6 @@ fn test_key_store() { // Master keys change after reset. We don't require this for ECDSA seeds because it's not the // case, but it might be better. key_store.reset().unwrap(); - assert!(key_store.kh_encryption().unwrap() != encryption_key); - assert!(key_store.kh_authentication().unwrap() != authentication_key); + assert!(key_store.key_handle_encryption().unwrap() != encryption_key); + assert!(key_store.key_handle_authentication().unwrap() != authentication_key); } diff --git a/src/ctap/crypto_wrapper.rs b/src/ctap/crypto_wrapper.rs index ea77968..a6a86c8 100644 --- a/src/ctap/crypto_wrapper.rs +++ b/src/ctap/crypto_wrapper.rs @@ -260,7 +260,7 @@ pub fn encrypt_key_handle( private_key: &PrivateKey, application: &[u8; 32], ) -> Result, Ctap2StatusCode> { - let aes_enc_key = crypto::aes256::EncryptionKey::new(&env.key_store().kh_encryption()?); + let aes_enc_key = crypto::aes256::EncryptionKey::new(&env.key_store().key_handle_encryption()?); let mut plaintext = [0; 64]; let version = match private_key { @@ -279,7 +279,10 @@ pub fn encrypt_key_handle( let mut encrypted_id = aes256_cbc_encrypt(env.rng(), &aes_enc_key, &plaintext, true)?; encrypted_id.insert(0, version); - let id_hmac = hmac_256::(&env.key_store().kh_authentication()?, &encrypted_id[..]); + let id_hmac = hmac_256::( + &env.key_store().key_handle_authentication()?, + &encrypted_id[..], + ); encrypted_id.extend(&id_hmac); Ok(encrypted_id) } @@ -305,7 +308,7 @@ pub fn decrypt_credential_source( } let hmac_message_size = credential_id.len() - 32; if !verify_hmac_256::( - &env.key_store().kh_authentication()?, + &env.key_store().key_handle_authentication()?, &credential_id[..hmac_message_size], array_ref![credential_id, hmac_message_size, 32], ) { @@ -329,7 +332,7 @@ pub fn decrypt_credential_source( return Ok(None); } - let aes_enc_key = crypto::aes256::EncryptionKey::new(&env.key_store().kh_encryption()?); + let aes_enc_key = crypto::aes256::EncryptionKey::new(&env.key_store().key_handle_encryption()?); let decrypted_id = aes256_cbc_decrypt(&aes_enc_key, payload, true)?; if rp_id_hash != &decrypted_id[32..] { @@ -599,7 +602,7 @@ mod test { encrypted_id[0] = UNSUPPORTED_CREDENTIAL_ID_VERSION; // Override the HMAC to pass the check. encrypted_id.truncate(&encrypted_id.len() - 32); - let hmac_key = env.key_store().kh_authentication().unwrap(); + let hmac_key = env.key_store().key_handle_authentication().unwrap(); let id_hmac = hmac_256::(&hmac_key, &encrypted_id[..]); encrypted_id.extend(&id_hmac); @@ -668,13 +671,17 @@ mod test { private_key: crypto::ecdsa::SecKey, application: &[u8; 32], ) -> Result, Ctap2StatusCode> { - let aes_enc_key = crypto::aes256::EncryptionKey::new(&env.key_store().kh_encryption()?); + let aes_enc_key = + crypto::aes256::EncryptionKey::new(&env.key_store().key_handle_encryption()?); let mut plaintext = [0; 64]; private_key.to_bytes(array_mut_ref!(plaintext, 0, 32)); plaintext[32..64].copy_from_slice(application); let mut encrypted_id = aes256_cbc_encrypt(env.rng(), &aes_enc_key, &plaintext, true)?; - let id_hmac = hmac_256::(&env.key_store().kh_authentication()?, &encrypted_id[..]); + let id_hmac = hmac_256::( + &env.key_store().key_handle_authentication()?, + &encrypted_id[..], + ); encrypted_id.extend(&id_hmac); Ok(encrypted_id) } From 87a4dc725f69ddd2f1f82443adda67ce5fcd96d7 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Thu, 30 Jun 2022 11:43:11 +0200 Subject: [PATCH 04/10] Fix doc --- src/api/key_store.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/key_store.rs b/src/api/key_store.rs index 5bbfae2..9723228 100644 --- a/src/api/key_store.rs +++ b/src/api/key_store.rs @@ -43,7 +43,7 @@ pub trait KeyStore { /// Key store errors. /// -/// They are deliberately indistinguishable from each other to avoid leaking information. +/// They are deliberately indistinguishable to avoid leaking information. #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub struct Error; From 2f9e82696d1fa0e8000b1d614009c1390b312dc6 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Thu, 30 Jun 2022 11:53:44 +0200 Subject: [PATCH 05/10] Fix ctap1 tests --- src/ctap/ctap1.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/ctap/ctap1.rs b/src/ctap/ctap1.rs index 2374a2a..5a72f77 100644 --- a/src/ctap/ctap1.rs +++ b/src/ctap/ctap1.rs @@ -502,7 +502,7 @@ mod test { let mut env = TestEnv::new(); env.user_presence() .set(|| panic!("Unexpected user presence check in CTAP1")); - let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256); + let sk = PrivateKey::new(&mut env, SignatureAlgorithm::ES256); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); let rp_id = "example.com"; @@ -520,7 +520,7 @@ mod test { let mut env = TestEnv::new(); env.user_presence() .set(|| panic!("Unexpected user presence check in CTAP1")); - let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256); + let sk = PrivateKey::new(&mut env, SignatureAlgorithm::ES256); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); let rp_id = "example.com"; @@ -539,7 +539,7 @@ mod test { let mut env = TestEnv::new(); env.user_presence() .set(|| panic!("Unexpected user presence check in CTAP1")); - let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256); + let sk = PrivateKey::new(&mut env, SignatureAlgorithm::ES256); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); let rp_id = "example.com"; @@ -577,7 +577,7 @@ mod test { let mut env = TestEnv::new(); env.user_presence() .set(|| panic!("Unexpected user presence check in CTAP1")); - let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256); + let sk = PrivateKey::new(&mut env, SignatureAlgorithm::ES256); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); let rp_id = "example.com"; @@ -597,7 +597,7 @@ mod test { let mut env = TestEnv::new(); env.user_presence() .set(|| panic!("Unexpected user presence check in CTAP1")); - let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256); + let sk = PrivateKey::new(&mut env, SignatureAlgorithm::ES256); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); let rp_id = "example.com"; @@ -617,7 +617,7 @@ mod test { let mut env = TestEnv::new(); env.user_presence() .set(|| panic!("Unexpected user presence check in CTAP1")); - let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256); + let sk = PrivateKey::new(&mut env, SignatureAlgorithm::ES256); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); let rp_id = "example.com"; @@ -645,7 +645,7 @@ mod test { let mut env = TestEnv::new(); env.user_presence() .set(|| panic!("Unexpected user presence check in CTAP1")); - let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256); + let sk = PrivateKey::new(&mut env, SignatureAlgorithm::ES256); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); let rp_id = "example.com"; @@ -673,7 +673,7 @@ mod test { let mut env = TestEnv::new(); env.user_presence() .set(|| panic!("Unexpected user presence check in CTAP1")); - let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256); + let sk = PrivateKey::new(&mut env, SignatureAlgorithm::ES256); let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0)); let rp_id = "example.com"; From 2256c739cd4080122c08edb9318ca51bfffc21f5 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Thu, 30 Jun 2022 12:04:46 +0200 Subject: [PATCH 06/10] Fix ed25519 test --- src/ctap/crypto_wrapper.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ctap/crypto_wrapper.rs b/src/ctap/crypto_wrapper.rs index a6a86c8..3df063a 100644 --- a/src/ctap/crypto_wrapper.rs +++ b/src/ctap/crypto_wrapper.rs @@ -446,7 +446,7 @@ mod test { #[cfg(feature = "ed25519")] fn test_new_ed25519_from_bytes() { let mut env = TestEnv::new(); - let private_key = PrivateKey::new(env.rng(), SignatureAlgorithm::EDDSA); + let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::EDDSA); let key_bytes = private_key.to_bytes(); assert_eq!( PrivateKey::new_ed25519_from_bytes(&key_bytes), From ff6c700cd9dad77ec5186005056c3419a40c5356 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Thu, 30 Jun 2022 15:03:38 +0200 Subject: [PATCH 07/10] Use indirection to implement the default KeyStore --- src/api/key_store.rs | 52 +++++++++++++++++++++++++------------------- src/env/test/mod.rs | 3 +++ src/env/tock/mod.rs | 3 +++ 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/api/key_store.rs b/src/api/key_store.rs index 9723228..f3714a2 100644 --- a/src/api/key_store.rs +++ b/src/api/key_store.rs @@ -50,7 +50,10 @@ pub struct Error; /// Key of the environment store reserved for the key store. pub const STORE_KEY: usize = 2046; -impl KeyStore for T { +/// Implements a default key store using the environment rng and store. +pub trait Helper: Env {} + +impl KeyStore for T { fn key_handle_encryption(&mut self) -> Result<[u8; 32], Error> { Ok(get_master_keys(self)?.encryption) } @@ -114,28 +117,33 @@ impl From for Error { } } -#[test] -fn test_key_store() { - let mut env = crate::env::test::TestEnv::new(); - let key_store = env.key_store(); +#[cfg(test)] +mod test { + use super::*; - // Master keys are well-defined and stable. - let encryption_key = key_store.key_handle_encryption().unwrap(); - let authentication_key = key_store.key_handle_authentication().unwrap(); - assert_eq!(key_store.key_handle_encryption(), Ok(encryption_key)); - assert_eq!( - key_store.key_handle_authentication(), - Ok(authentication_key) - ); + #[test] + fn test_key_store() { + let mut env = crate::env::test::TestEnv::new(); + let key_store = env.key_store(); - // ECDSA seeds are well-defined and stable. - let ecdsa_seed = key_store.generate_ecdsa_seed().unwrap(); - let ecdsa_key = key_store.derive_ecdsa(&ecdsa_seed).unwrap(); - assert_eq!(key_store.derive_ecdsa(&ecdsa_seed), Ok(ecdsa_key)); + // Master keys are well-defined and stable. + let encryption_key = key_store.key_handle_encryption().unwrap(); + let authentication_key = key_store.key_handle_authentication().unwrap(); + assert_eq!(key_store.key_handle_encryption(), Ok(encryption_key)); + assert_eq!( + key_store.key_handle_authentication(), + Ok(authentication_key) + ); - // Master keys change after reset. We don't require this for ECDSA seeds because it's not the - // case, but it might be better. - key_store.reset().unwrap(); - assert!(key_store.key_handle_encryption().unwrap() != encryption_key); - assert!(key_store.key_handle_authentication().unwrap() != authentication_key); + // ECDSA seeds are well-defined and stable. + let ecdsa_seed = key_store.generate_ecdsa_seed().unwrap(); + let ecdsa_key = key_store.derive_ecdsa(&ecdsa_seed).unwrap(); + assert_eq!(key_store.derive_ecdsa(&ecdsa_seed), Ok(ecdsa_key)); + + // Master keys change after reset. We don't require this for ECDSA seeds because it's not + // the case, but it might be better. + key_store.reset().unwrap(); + assert!(key_store.key_handle_encryption().unwrap() != encryption_key); + assert!(key_store.key_handle_authentication().unwrap() != authentication_key); + } } diff --git a/src/env/test/mod.rs b/src/env/test/mod.rs index ea5f2b0..1cd6d24 100644 --- a/src/env/test/mod.rs +++ b/src/env/test/mod.rs @@ -16,6 +16,7 @@ use self::upgrade_storage::BufferUpgradeStorage; use crate::api::connection::{HidConnection, SendOrRecvResult, SendOrRecvStatus}; use crate::api::customization::DEFAULT_CUSTOMIZATION; use crate::api::firmware_protection::FirmwareProtection; +use crate::api::key_store; use crate::api::user_presence::{UserPresence, UserPresenceResult}; use crate::clock::ClockInt; use crate::env::Env; @@ -147,6 +148,8 @@ impl FirmwareProtection for TestEnv { } } +impl key_store::Helper for TestEnv {} + impl Env for TestEnv { type Rng = TestRng256; type UserPresence = TestUserPresence; diff --git a/src/env/tock/mod.rs b/src/env/tock/mod.rs index e76088f..8cb6274 100644 --- a/src/env/tock/mod.rs +++ b/src/env/tock/mod.rs @@ -16,6 +16,7 @@ pub use self::storage::{TockStorage, TockUpgradeStorage}; use crate::api::connection::{HidConnection, SendOrRecvError, SendOrRecvResult, SendOrRecvStatus}; use crate::api::customization::{CustomizationImpl, DEFAULT_CUSTOMIZATION}; use crate::api::firmware_protection::FirmwareProtection; +use crate::api::key_store; use crate::api::user_presence::{UserPresence, UserPresenceError, UserPresenceResult}; use crate::clock::{ClockInt, KEEPALIVE_DELAY_MS}; use crate::env::Env; @@ -193,6 +194,8 @@ impl FirmwareProtection for TockEnv { } } +impl key_store::Helper for TockEnv {} + impl Env for TockEnv { type Rng = TockRng256; type UserPresence = Self; From 30a3205fa7ccae053f3811ac341a0410a562ed30 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Fri, 1 Jul 2022 10:58:56 +0200 Subject: [PATCH 08/10] Address comments --- src/ctap/crypto_wrapper.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/ctap/crypto_wrapper.rs b/src/ctap/crypto_wrapper.rs index 3df063a..6a35bd1 100644 --- a/src/ctap/crypto_wrapper.rs +++ b/src/ctap/crypto_wrapper.rs @@ -155,7 +155,7 @@ impl PrivateKey { /// Returns the ECDSA private key. pub fn ecdsa_key(&self, env: &mut impl Env) -> Option { match self { - PrivateKey::Ecdsa(seed) => ecdsa_key(env, seed), + PrivateKey::Ecdsa(seed) => ecdsa_key_from_seed(env, seed), #[allow(unreachable_patterns)] _ => None, } @@ -164,7 +164,9 @@ impl PrivateKey { /// Returns the corresponding public key. pub fn get_pub_key(&self, env: &mut impl Env) -> Option { Some(match self { - PrivateKey::Ecdsa(ecdsa_seed) => CoseKey::from(ecdsa_key(env, ecdsa_seed)?.genpk()), + PrivateKey::Ecdsa(ecdsa_seed) => { + CoseKey::from(ecdsa_key_from_seed(env, ecdsa_seed)?.genpk()) + } #[cfg(feature = "ed25519")] PrivateKey::Ed25519(ed25519_key) => CoseKey::from(ed25519_key.public_key()), }) @@ -173,7 +175,7 @@ impl PrivateKey { /// Returns the encoded signature for a given message. pub fn sign_and_encode(&self, env: &mut impl Env, message: &[u8]) -> Option> { Some(match self { - PrivateKey::Ecdsa(ecdsa_seed) => ecdsa_key(env, ecdsa_seed)? + PrivateKey::Ecdsa(ecdsa_seed) => ecdsa_key_from_seed(env, ecdsa_seed)? .sign_rfc6979::(message) .to_asn1_der(), #[cfg(feature = "ed25519")] @@ -200,7 +202,7 @@ impl PrivateKey { } } -fn ecdsa_key(env: &mut impl Env, seed: &[u8; 32]) -> Option { +fn ecdsa_key_from_seed(env: &mut impl Env, seed: &[u8; 32]) -> Option { let ecdsa_bytes = env.key_store().derive_ecdsa(seed).ok()?; ecdsa::SecKey::from_bytes(&ecdsa_bytes) } From ecb98b0f58d9b6365d103fedc9f8a15c353473ec Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Fri, 1 Jul 2022 13:26:35 +0200 Subject: [PATCH 09/10] Return Result instead of Option --- src/ctap/credential_management.rs | 4 +--- src/ctap/crypto_wrapper.rs | 29 ++++++++++++++++++----------- src/ctap/ctap1.rs | 4 ++-- src/ctap/mod.rs | 14 +++----------- 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/src/ctap/credential_management.rs b/src/ctap/credential_management.rs index 27fc5a7..caf4aa8 100644 --- a/src/ctap/credential_management.rs +++ b/src/ctap/credential_management.rs @@ -93,9 +93,7 @@ fn enumerate_credentials_response( key_id: credential_id, transports: None, // You can set USB as a hint here. }; - let public_key = private_key - .get_pub_key(env) - .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?; + let public_key = private_key.get_pub_key(env)?; Ok(AuthenticatorCredentialManagementResponse { user: Some(user), credential_id: Some(credential_id), diff --git a/src/ctap/crypto_wrapper.rs b/src/ctap/crypto_wrapper.rs index 6a35bd1..881b075 100644 --- a/src/ctap/crypto_wrapper.rs +++ b/src/ctap/crypto_wrapper.rs @@ -153,17 +153,17 @@ impl PrivateKey { } /// Returns the ECDSA private key. - pub fn ecdsa_key(&self, env: &mut impl Env) -> Option { + pub fn ecdsa_key(&self, env: &mut impl Env) -> Result { match self { PrivateKey::Ecdsa(seed) => ecdsa_key_from_seed(env, seed), #[allow(unreachable_patterns)] - _ => None, + _ => Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR), } } /// Returns the corresponding public key. - pub fn get_pub_key(&self, env: &mut impl Env) -> Option { - Some(match self { + pub fn get_pub_key(&self, env: &mut impl Env) -> Result { + Ok(match self { PrivateKey::Ecdsa(ecdsa_seed) => { CoseKey::from(ecdsa_key_from_seed(env, ecdsa_seed)?.genpk()) } @@ -173,8 +173,12 @@ impl PrivateKey { } /// Returns the encoded signature for a given message. - pub fn sign_and_encode(&self, env: &mut impl Env, message: &[u8]) -> Option> { - Some(match self { + pub fn sign_and_encode( + &self, + env: &mut impl Env, + message: &[u8], + ) -> Result, Ctap2StatusCode> { + Ok(match self { PrivateKey::Ecdsa(ecdsa_seed) => ecdsa_key_from_seed(env, ecdsa_seed)? .sign_rfc6979::(message) .to_asn1_der(), @@ -202,9 +206,12 @@ impl PrivateKey { } } -fn ecdsa_key_from_seed(env: &mut impl Env, seed: &[u8; 32]) -> Option { - let ecdsa_bytes = env.key_store().derive_ecdsa(seed).ok()?; - ecdsa::SecKey::from_bytes(&ecdsa_bytes) +fn ecdsa_key_from_seed( + env: &mut impl Env, + seed: &[u8; 32], +) -> Result { + let ecdsa_bytes = env.key_store().derive_ecdsa(seed)?; + Ok(ecdsa::SecKey::from_bytes(&ecdsa_bytes).unwrap()) } impl From for cbor::Value { @@ -481,7 +488,7 @@ mod test { let public_key = ecdsa_key.genpk(); assert_eq!( private_key.get_pub_key(&mut env), - Some(CoseKey::from(public_key)) + Ok(CoseKey::from(public_key)) ); } @@ -494,7 +501,7 @@ mod test { let signature = ecdsa_key.sign_rfc6979::(&message).to_asn1_der(); assert_eq!( private_key.sign_and_encode(&mut env, &message), - Some(signature) + Ok(signature) ); } diff --git a/src/ctap/ctap1.rs b/src/ctap/ctap1.rs index 5a72f77..00def87 100644 --- a/src/ctap/ctap1.rs +++ b/src/ctap/ctap1.rs @@ -248,7 +248,7 @@ impl Ctap1Command { let private_key = PrivateKey::new_ecdsa(env); let sk = private_key .ecdsa_key(env) - .ok_or(Ctap1StatusCode::SW_INTERNAL_EXCEPTION)?; + .map_err(|_| Ctap1StatusCode::SW_INTERNAL_EXCEPTION)?; let pk = sk.genpk(); let key_handle = encrypt_key_handle(env, &private_key, &application) .map_err(|_| Ctap1StatusCode::SW_INTERNAL_EXCEPTION)?; @@ -315,7 +315,7 @@ impl Ctap1Command { let ecdsa_key = credential_source .private_key .ecdsa_key(env) - .ok_or(Ctap1StatusCode::SW_WRONG_DATA)?; + .map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?; if flags == Ctap1Flags::CheckOnly { return Err(Ctap1StatusCode::SW_COND_USE_NOT_SATISFIED); } diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 678cc29..52a410f 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -892,9 +892,7 @@ impl CtapState { } auth_data.extend(vec![0x00, credential_id.len() as u8]); auth_data.extend(&credential_id); - let public_cose_key = private_key - .get_pub_key(env) - .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?; + let public_cose_key = private_key.get_pub_key(env)?; cbor_write(cbor::Value::from(public_cose_key), &mut auth_data)?; if has_extension_output { let hmac_secret_output = if extensions.hmac_secret { @@ -934,12 +932,7 @@ impl CtapState { Some(vec![attestation_certificate]), ) } else { - ( - private_key - .sign_and_encode(env, &signature_data) - .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?, - None, - ) + (private_key.sign_and_encode(env, &signature_data)?, None) }; let attestation_statement = PackedAttestationStatement { alg: SignatureAlgorithm::ES256 as i64, @@ -1023,8 +1016,7 @@ impl CtapState { signature_data.extend(client_data_hash); let signature = credential .private_key - .sign_and_encode(env, &signature_data) - .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?; + .sign_and_encode(env, &signature_data)?; let cred_desc = PublicKeyCredentialDescriptor { key_type: PublicKeyCredentialType::PublicKey, From c8dc1dd0e850a63704b55bd71f71a62671ab0202 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Fri, 1 Jul 2022 13:59:09 +0200 Subject: [PATCH 10/10] Rename STORE_KEY --- src/api/key_store.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/api/key_store.rs b/src/api/key_store.rs index f3714a2..f40bc9b 100644 --- a/src/api/key_store.rs +++ b/src/api/key_store.rs @@ -21,7 +21,7 @@ use crate::env::Env; /// Provides storage for secret keys. /// -/// Implementations may use the environment store: [`STORE_KEY`] is reserved for this usage. +/// Implementations may use the environment store: [`STORAGE_KEY`] is reserved for this usage. pub trait KeyStore { /// Returns the AES key for key handles encryption. fn key_handle_encryption(&mut self) -> Result<[u8; 32], Error>; @@ -48,7 +48,7 @@ pub trait KeyStore { pub struct Error; /// Key of the environment store reserved for the key store. -pub const STORE_KEY: usize = 2046; +pub const STORAGE_KEY: usize = 2046; /// Implements a default key store using the environment rng and store. pub trait Helper: Env {} @@ -76,7 +76,7 @@ impl KeyStore for T { } fn reset(&mut self) -> Result<(), Error> { - Ok(self.store().remove(STORE_KEY)?) + Ok(self.store().remove(STORAGE_KEY)?) } } @@ -90,7 +90,7 @@ struct MasterKeys { } fn get_master_keys(env: &mut impl Env) -> Result { - let master_keys = match env.store().find(STORE_KEY)? { + let master_keys = match env.store().find(STORAGE_KEY)? { Some(x) => x, None => { let master_encryption_key = env.rng().gen_uniform_u8x32(); @@ -98,7 +98,7 @@ fn get_master_keys(env: &mut impl Env) -> Result { let mut master_keys = Vec::with_capacity(64); master_keys.extend_from_slice(&master_encryption_key); master_keys.extend_from_slice(&master_authentication_key); - env.store().insert(STORE_KEY, &master_keys)?; + env.store().insert(STORAGE_KEY, &master_keys)?; master_keys } };