From e3d2e7d77809eb45ee736b12e4113f8219cca3cf Mon Sep 17 00:00:00 2001 From: Jean-Michel Picod Date: Wed, 26 Jul 2023 14:21:55 +0200 Subject: [PATCH 1/3] Fix USB enumeration on OS X (#640) Hopefully without breaking the others. Summary of the changes: - Device descriptor reports the device is bus powered and requires 100mA max. - HID descriptor version bumped to 1.11 (was 1.10) - Added string index for Interface and HID descriptors (which seems to make OS X happy) --- .../nordic/nrf52840_dongle_opensk/src/main.rs | 4 +++ boards/nordic/nrf52840_mdk_dfu/src/main.rs | 4 +++ boards/nordic/nrf52840dk_opensk/src/main.rs | 4 +++ patches/tock/03-add-ctap-modules.patch | 27 +++++++++++++++---- patches/tock/04-vendor-hid.patch | 2 +- patches/tock/06-persistent-storage.patch | 4 +-- 6 files changed, 37 insertions(+), 8 deletions(-) diff --git a/boards/nordic/nrf52840_dongle_opensk/src/main.rs b/boards/nordic/nrf52840_dongle_opensk/src/main.rs index 1761bf02..bbbeda7b 100644 --- a/boards/nordic/nrf52840_dongle_opensk/src/main.rs +++ b/boards/nordic/nrf52840_dongle_opensk/src/main.rs @@ -55,6 +55,10 @@ static STRINGS: &'static [&'static str] = &[ "OpenSK", // Serial number "v1.0", + // Interface description + main HID string + "FIDO2", + // vendor HID string + "Vendor HID", ]; // State for loading and holding applications. diff --git a/boards/nordic/nrf52840_mdk_dfu/src/main.rs b/boards/nordic/nrf52840_mdk_dfu/src/main.rs index e599d469..de479262 100644 --- a/boards/nordic/nrf52840_mdk_dfu/src/main.rs +++ b/boards/nordic/nrf52840_mdk_dfu/src/main.rs @@ -48,6 +48,10 @@ static STRINGS: &'static [&'static str] = &[ "OpenSK", // Serial number "v1.0", + // Interface description + main HID string + "FIDO2", + // vendor HID string + "Vendor HID", ]; // State for loading and holding applications. diff --git a/boards/nordic/nrf52840dk_opensk/src/main.rs b/boards/nordic/nrf52840dk_opensk/src/main.rs index 21d22c0d..41519d09 100644 --- a/boards/nordic/nrf52840dk_opensk/src/main.rs +++ b/boards/nordic/nrf52840dk_opensk/src/main.rs @@ -119,6 +119,10 @@ static STRINGS: &'static [&'static str] = &[ "OpenSK", // Serial number "v1.0", + // Interface description + main HID string + "FIDO2", + // vendor HID string + "Vendor HID", ]; // State for loading and holding applications. diff --git a/patches/tock/03-add-ctap-modules.patch b/patches/tock/03-add-ctap-modules.patch index 5ffd5eb6..d4adb6e9 100644 --- a/patches/tock/03-add-ctap-modules.patch +++ b/patches/tock/03-add-ctap-modules.patch @@ -180,6 +180,21 @@ index 000000000..f2e248bc9 + self.side == Some(side) + } +} +diff --git a/capsules/src/usb/descriptors.rs b/capsules/src/usb/descriptors.rs +index 67f708239..9c5bc9cd1 100644 +--- a/capsules/src/usb/descriptors.rs ++++ b/capsules/src/usb/descriptors.rs +@@ -568,8 +568,8 @@ impl Default for ConfigurationDescriptor { + num_interfaces: 1, + configuration_value: 1, + string_index: 0, +- attributes: ConfigurationAttributes::new(true, false), +- max_power: 0, // in 2mA units ++ attributes: ConfigurationAttributes::new(false, false), ++ max_power: 50, // in 2mA units + related_descriptor_length: 0, + } + } diff --git a/capsules/src/usb/mod.rs b/capsules/src/usb/mod.rs index 767f5de83..cb5e0af97 100644 --- a/capsules/src/usb/mod.rs @@ -544,10 +559,10 @@ index 000000000..30cac1323 +} diff --git a/capsules/src/usb/usbc_ctap_hid.rs b/capsules/src/usb/usbc_ctap_hid.rs new file mode 100644 -index 000000000..e074eb7a6 +index 000000000..5ad2c44b3 --- /dev/null +++ b/capsules/src/usb/usbc_ctap_hid.rs -@@ -0,0 +1,552 @@ +@@ -0,0 +1,554 @@ +//! A USB HID client of the USB hardware interface + +use super::app::App; @@ -652,14 +667,14 @@ index 000000000..e074eb7a6 + }]; + +static HID: HIDDescriptor<'static> = HIDDescriptor { -+ hid_class: 0x0110, ++ hid_class: 0x0111, + country_code: HIDCountryCode::NotSupported, + sub_descriptors: HID_SUB_DESCRIPTORS, +}; + +#[cfg(feature = "vendor_hid")] +static VENDOR_HID: HIDDescriptor<'static> = HIDDescriptor { -+ hid_class: 0x0110, ++ hid_class: 0x0111, + country_code: HIDCountryCode::NotSupported, + sub_descriptors: VENDOR_HID_SUB_DESCRIPTORS, +}; @@ -719,6 +734,7 @@ index 000000000..e074eb7a6 + interface_class: 0x03, // HID + interface_subclass: 0x00, // no subcall + interface_protocol: 0x00, // no protocol ++ string_index: 4, + ..InterfaceDescriptor::default() + }, + // Vendor HID interface. @@ -728,6 +744,7 @@ index 000000000..e074eb7a6 + interface_class: 0x03, // HID + interface_subclass: 0x00, + interface_protocol: 0x00, ++ string_index: 5, + ..InterfaceDescriptor::default() + }, + ]; @@ -786,7 +803,7 @@ index 000000000..e074eb7a6 + manufacturer_string: 1, + product_string: 2, + serial_number_string: 3, -+ class: 0x03, // HID class for all interfaces ++ class: 0x00, // Class is specified at the interface level + max_packet_size_ep0: max_ctrl_packet_size, + ..descriptors::DeviceDescriptor::default() + }, diff --git a/patches/tock/04-vendor-hid.patch b/patches/tock/04-vendor-hid.patch index 7f65e3f1..eb7af004 100644 --- a/patches/tock/04-vendor-hid.patch +++ b/patches/tock/04-vendor-hid.patch @@ -10,7 +10,7 @@ index 65301bcf1..dc70e98b1 100644 +[features] +vendor_hid = [] diff --git a/capsules/src/usb/descriptors.rs b/capsules/src/usb/descriptors.rs -index 67f708239..c2556d3a2 100644 +index 9c5bc9cd1..c3ed71c44 100644 --- a/capsules/src/usb/descriptors.rs +++ b/capsules/src/usb/descriptors.rs @@ -415,13 +415,14 @@ impl DescriptorBuffer { diff --git a/patches/tock/06-persistent-storage.patch b/patches/tock/06-persistent-storage.patch index d769bfa8..3921a2fb 100644 --- a/patches/tock/06-persistent-storage.patch +++ b/patches/tock/06-persistent-storage.patch @@ -1,5 +1,5 @@ diff --git a/chips/nrf52/src/nvmc.rs b/chips/nrf52/src/nvmc.rs -index 61e94260e..b7c3be3f6 100644 +index 61e94260e..e115e1851 100644 --- a/chips/nrf52/src/nvmc.rs +++ b/chips/nrf52/src/nvmc.rs @@ -5,7 +5,14 @@ @@ -393,7 +393,7 @@ index 028f30220..8880bc000 100644 pub use crate::process::ProcessId; pub use crate::scheduler::Scheduler; diff --git a/kernel/src/memop.rs b/kernel/src/memop.rs -index 51d89f37c..45ab3856b 100644 +index 51d89f37c..c4f7cef92 100644 --- a/kernel/src/memop.rs +++ b/kernel/src/memop.rs @@ -107,6 +107,37 @@ pub(crate) fn memop(process: &dyn Process, op_type: usize, r1: usize) -> Syscall From 96af5e81a5f84e38be7990db3b3e7ac4f946592c Mon Sep 17 00:00:00 2001 From: Zach Halvorsen Date: Wed, 9 Aug 2023 08:48:05 -0700 Subject: [PATCH 2/3] Add more transparency into some EC structures. (#641) This adds the ability to create ECDH keys from raw bytes and export signatures as raw bytes. --- libraries/crypto/src/ecdh.rs | 11 +++++++++++ libraries/crypto/src/ecdsa.rs | 5 +---- libraries/opensk/src/api/crypto/ecdsa.rs | 1 - libraries/opensk/src/api/crypto/software_crypto.rs | 1 - 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/libraries/crypto/src/ecdh.rs b/libraries/crypto/src/ecdh.rs index ed5bec03..dc7f0583 100644 --- a/libraries/crypto/src/ecdh.rs +++ b/libraries/crypto/src/ecdh.rs @@ -78,6 +78,17 @@ impl SecKey { p.getx().to_int().to_bin(&mut x); x } + + /// Creates a private key from the exponent's bytes, or None if checks fail. + pub fn from_bytes(bytes: &[u8; 32]) -> Option { + let a = NonZeroExponentP256::from_int_checked(Int256::from_bin(bytes)); + // The branching here is fine because all this reveals is whether the key was invalid. + if bool::from(a.is_none()) { + return None; + } + let a = a.unwrap(); + Some(SecKey { a }) + } } impl PubKey { diff --git a/libraries/crypto/src/ecdsa.rs b/libraries/crypto/src/ecdsa.rs index f4e66211..ca29b8da 100644 --- a/libraries/crypto/src/ecdsa.rs +++ b/libraries/crypto/src/ecdsa.rs @@ -19,9 +19,7 @@ use super::ec::point::PointP256; use super::Hash256; use alloc::vec; use alloc::vec::Vec; -#[cfg(feature = "std")] -use arrayref::array_mut_ref; -use arrayref::{array_ref, mut_array_refs}; +use arrayref::{array_mut_ref, array_ref, mut_array_refs}; use core::marker::PhantomData; use rand_core::RngCore; use zeroize::Zeroize; @@ -220,7 +218,6 @@ impl Signature { Some(Signature { r, s }) } - #[cfg(feature = "std")] pub fn to_bytes(&self, bytes: &mut [u8; Signature::BYTES_LENGTH]) { self.r .to_int() diff --git a/libraries/opensk/src/api/crypto/ecdsa.rs b/libraries/opensk/src/api/crypto/ecdsa.rs index 1dad533b..6c76e12b 100644 --- a/libraries/opensk/src/api/crypto/ecdsa.rs +++ b/libraries/opensk/src/api/crypto/ecdsa.rs @@ -73,7 +73,6 @@ pub trait Signature: Sized { fn from_slice(bytes: &[u8; EC_SIGNATURE_SIZE]) -> Option; /// Writes the signature bytes into the passed in parameter. - #[cfg(feature = "std")] fn to_slice(&self, bytes: &mut [u8; EC_SIGNATURE_SIZE]); /// Encodes the signatures as ASN1 DER. diff --git a/libraries/opensk/src/api/crypto/software_crypto.rs b/libraries/opensk/src/api/crypto/software_crypto.rs index f0cd2216..6ce9bbb1 100644 --- a/libraries/opensk/src/api/crypto/software_crypto.rs +++ b/libraries/opensk/src/api/crypto/software_crypto.rs @@ -169,7 +169,6 @@ impl ecdsa::Signature for SoftwareEcdsaSignature { crypto::ecdsa::Signature::from_bytes(bytes).map(|s| SoftwareEcdsaSignature { signature: s }) } - #[cfg(feature = "std")] fn to_slice(&self, bytes: &mut [u8; EC_SIGNATURE_SIZE]) { self.signature.to_bytes(bytes); } From 87f07112845b1e4f9116d7b9a940a61399657f8d Mon Sep 17 00:00:00 2001 From: kaczmarczyck <43844792+kaczmarczyck@users.noreply.github.com> Date: Thu, 10 Aug 2023 14:20:39 +0200 Subject: [PATCH 3/3] New key wrapping API (#642) * New key wrapping API Allows key wrapping to be different between persistent and server-side storage. To accomplish this, the PrivateKey now always stores the fully reconstructed key, and has different methods to serialize it for the respective use case. * Cleans up legacy credential parsing This is a backwards incompatible change. This PR already introduces backwards incompatible new credential parsing, and therefore we can also remove all other legacy parsing. * Correct credential minimum size * wider public interface to allow custom PrivateKey construction --- libraries/opensk/src/api/key_store.rs | 172 +++++---------- libraries/opensk/src/api/private_key.rs | 114 +++++----- .../opensk/src/ctap/credential_management.rs | 9 +- libraries/opensk/src/ctap/crypto_wrapper.rs | 21 +- libraries/opensk/src/ctap/ctap1.rs | 13 +- libraries/opensk/src/ctap/data_formats.rs | 205 +++++++----------- libraries/opensk/src/ctap/mod.rs | 6 +- libraries/opensk/src/ctap/pin_protocol.rs | 4 +- libraries/opensk/src/ctap/storage.rs | 58 +++-- 9 files changed, 253 insertions(+), 349 deletions(-) diff --git a/libraries/opensk/src/api/key_store.rs b/libraries/opensk/src/api/key_store.rs index de150fd1..7b966dc0 100644 --- a/libraries/opensk/src/api/key_store.rs +++ b/libraries/opensk/src/api/key_store.rs @@ -13,7 +13,6 @@ // limitations under the License. use crate::api::crypto::aes256::Aes256; -use crate::api::crypto::ecdsa::SecretKey as _; use crate::api::crypto::hmac256::Hmac256; use crate::api::crypto::HASH_SIZE; use crate::api::private_key::PrivateKey; @@ -21,7 +20,7 @@ use crate::ctap::crypto_wrapper::{aes256_cbc_decrypt, aes256_cbc_encrypt}; use crate::ctap::data_formats::CredentialProtectionPolicy; use crate::ctap::secret::Secret; use crate::ctap::{cbor_read, cbor_write}; -use crate::env::{AesKey, EcdsaSk, Env, Hmac}; +use crate::env::{AesKey, Env, Hmac}; use alloc::vec; use alloc::vec::Vec; use core::convert::{TryFrom, TryInto}; @@ -30,14 +29,13 @@ use rand_core::RngCore; use sk_cbor as cbor; use sk_cbor::{cbor_map_options, destructure_cbor_map}; -const LEGACY_CREDENTIAL_ID_SIZE: usize = 112; // CBOR credential IDs consist of // - 1 byte : version number // - 16 bytes: initialization vector for AES-256, // - 192 bytes: encrypted block of the key handle cbor, // - 32 bytes: HMAC-SHA256 over everything else. pub const CBOR_CREDENTIAL_ID_SIZE: usize = 241; -const MIN_CREDENTIAL_ID_SIZE: usize = LEGACY_CREDENTIAL_ID_SIZE; +const MIN_CREDENTIAL_ID_SIZE: usize = CBOR_CREDENTIAL_ID_SIZE; pub(crate) const MAX_CREDENTIAL_ID_SIZE: usize = CBOR_CREDENTIAL_ID_SIZE; pub const CBOR_CREDENTIAL_ID_VERSION: u8 = 0x01; @@ -76,6 +74,14 @@ pub trait KeyStore { /// This function should be a no-op if the key store is already initialized. fn init(&mut self) -> Result<(), Error>; + /// Key to wrap (secret) data. + /// + /// Useful for encrypting data before + /// - writing it to persistent storage, + /// - CBOR encoding it, + /// - doing anything that does not support [`Secret`]. + fn wrap_key(&mut self) -> Result, Error>; + /// Encodes a credential as a binary strings. /// /// The output is encrypted and authenticated. Since the wrapped credentials are passed to the @@ -109,14 +115,6 @@ pub trait KeyStore { /// Decrypts a PIN hash. fn decrypt_pin_hash(&mut self, cipher: &[u8; 16]) -> Result, Error>; - /// Derives an ECDSA private key from a seed. - /// - /// The result is big-endian. - fn derive_ecdsa(&mut self, seed: &[u8; 32]) -> Result, Error>; - - /// Generates a seed to derive an ECDSA private key. - fn generate_ecdsa_seed(&mut self) -> Result, Error>; - /// Resets the key store. fn reset(&mut self) -> Result<(), Error>; } @@ -138,14 +136,23 @@ impl KeyStore for T { Ok(()) } + fn wrap_key(&mut self) -> Result, Error> { + Ok(AesKey::::new(&get_master_keys(self)?.encryption)) + } + /// Encrypts the given credential source data into a credential ID. /// /// Other information, such as a user name, are not stored. Since encrypted credential IDs are /// stored server-side, this information is already available (unencrypted). fn wrap_credential(&mut self, credential: CredentialSource) -> Result, Error> { let mut payload = Vec::new(); + let wrap_key = self.wrap_key::()?; + let private_key_cbor = credential + .private_key + .to_cbor::(self.rng(), &wrap_key) + .map_err(|_| Error)?; let cbor = cbor_map_options! { - CredentialSourceField::PrivateKey => credential.private_key, + CredentialSourceField::PrivateKey => private_key_cbor, CredentialSourceField::RpIdHash => credential.rp_id_hash, CredentialSourceField::CredProtectPolicy => credential.cred_protect_policy, CredentialSourceField::CredBlob => credential.cred_blob, @@ -155,7 +162,7 @@ impl KeyStore for T { let master_keys = get_master_keys(self)?; let aes_key = AesKey::::new(&master_keys.encryption); let encrypted_payload = - aes256_cbc_encrypt(self, &aes_key, &payload, true).map_err(|_| Error)?; + aes256_cbc_encrypt::(self.rng(), &aes_key, &payload, true).map_err(|_| Error)?; let mut credential_id = encrypted_payload; credential_id.insert(0, CBOR_CREDENTIAL_ID_VERSION); @@ -175,12 +182,6 @@ impl KeyStore for T { /// - the format does not match any known versions, or /// - the HMAC test fails. /// - /// For v0 (legacy U2F) the credential ID consists of: - /// - 16 bytes: initialization vector for AES-256, - /// - 32 bytes: encrypted ECDSA private key for the credential, - /// - 32 bytes: encrypted relying party ID hashed with SHA256, - /// - 32 bytes: HMAC-SHA256 over everything else. - /// /// For v1 (CBOR) the credential ID consists of: /// - 1 byte : version number, /// - 16 bytes: initialization vector for AES-256, @@ -204,21 +205,18 @@ impl KeyStore for T { return Ok(None); } - let credential_source = if bytes.len() == LEGACY_CREDENTIAL_ID_SIZE { - decrypt_legacy_credential_id::(&master_keys.encryption, &bytes[..hmac_message_size])? - } else { - match bytes[0] { - CBOR_CREDENTIAL_ID_VERSION => { - if bytes.len() != CBOR_CREDENTIAL_ID_SIZE { - return Ok(None); - } - decrypt_cbor_credential_id::( - &master_keys.encryption, - &bytes[1..hmac_message_size], - )? + let credential_source = match bytes[0] { + CBOR_CREDENTIAL_ID_VERSION => { + if bytes.len() != CBOR_CREDENTIAL_ID_SIZE { + return Ok(None); } - _ => return Ok(None), + decrypt_cbor_credential_id::( + self, + &master_keys.encryption, + &bytes[1..hmac_message_size], + )? } + _ => return Ok(None), }; if let Some(credential_source) = &credential_source { @@ -241,24 +239,8 @@ impl KeyStore for T { Ok(Secret::from_exposed_secret(*cipher)) } - fn derive_ecdsa(&mut self, seed: &[u8; 32]) -> Result, Error> { - match EcdsaSk::::from_slice(seed) { - None => Err(Error), - Some(_) => { - let mut derived: Secret<[u8; 32]> = Secret::default(); - derived.copy_from_slice(seed); - Ok(derived) - } - } - } - - fn generate_ecdsa_seed(&mut self) -> Result, Error> { - let mut seed = Secret::default(); - EcdsaSk::::random(self.rng()).to_slice(&mut seed); - Ok(seed) - } - fn reset(&mut self) -> Result<(), Error> { + // The storage also removes `STORAGE_KEY`, but this makes KeyStore more self-sufficient. Ok(self.store().remove(STORAGE_KEY)?) } } @@ -333,31 +315,8 @@ fn remove_padding(data: &[u8]) -> Result<&[u8], Error> { Ok(&data[..data.len() - pad_length as usize]) } -fn decrypt_legacy_credential_id( - encryption_key_bytes: &[u8; 32], - bytes: &[u8], -) -> Result, Error> { - let aes_key = AesKey::::new(encryption_key_bytes); - let plaintext = aes256_cbc_decrypt::(&aes_key, bytes, true) - .map_err(|_| Error)? - .expose_secret_to_vec(); - if plaintext.len() != 64 { - return Ok(None); - } - let private_key = if let Some(key) = PrivateKey::new_ecdsa_from_bytes(&plaintext[..32]) { - key - } else { - return Ok(None); - }; - Ok(Some(CredentialSource { - private_key, - rp_id_hash: plaintext[32..64].try_into().unwrap(), - cred_protect_policy: None, - cred_blob: None, - })) -} - fn decrypt_cbor_credential_id( + env: &mut E, encryption_key_bytes: &[u8; 32], bytes: &[u8], ) -> Result, Error> { @@ -376,7 +335,9 @@ fn decrypt_cbor_credential_id( } Ok(match (private_key, rp_id_hash) { (Some(private_key), Some(rp_id_hash)) => { - let private_key = PrivateKey::try_from(private_key).map_err(|_| Error)?; + let wrap_key = env.key_store().wrap_key::()?; + let private_key = + PrivateKey::from_cbor::(&wrap_key, private_key).map_err(|_| Error)?; let rp_id_hash = extract_byte_string(rp_id_hash)?; if rp_id_hash.len() != 32 { return Err(Error); @@ -420,9 +381,11 @@ fn extract_map(cbor_value: cbor::Value) -> Result().unwrap(); + let mut test_block = [0x33; 16]; + wrap_key.encrypt_block(&mut test_block); + let new_wrap_key = key_store.wrap_key::().unwrap(); + let mut new_test_block = [0x33; 16]; + new_wrap_key.encrypt_block(&mut new_test_block); + assert_eq!(&new_test_block, &test_block); // 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_ne!(&key_store.cred_random(false).unwrap(), &cred_random_no_uv); assert_ne!(&key_store.cred_random(true).unwrap(), &cred_random_with_uv); + let new_wrap_key = key_store.wrap_key::().unwrap(); + let mut new_test_block = [0x33; 16]; + new_wrap_key.encrypt_block(&mut new_test_block); + assert_ne!(&new_test_block, &test_block); } #[test] @@ -585,49 +556,6 @@ mod test { test_wrap_unwrap_credential_missing_blocks(SignatureAlgorithm::Eddsa); } - /// This is a copy of the function that genereated deprecated key handles. - fn legacy_encrypt_to_credential_id( - env: &mut TestEnv, - private_key: EcdsaSk, - application: &[u8; 32], - ) -> Result, Error> { - let master_keys = get_master_keys(env).unwrap(); - let aes_key = AesKey::::new(&master_keys.encryption); - let hmac_key = master_keys.authentication; - let mut plaintext = [0; 64]; - private_key.to_slice(array_mut_ref!(plaintext, 0, 32)); - plaintext[32..64].copy_from_slice(application); - - let mut encrypted_id = - aes256_cbc_encrypt(env, &aes_key, &plaintext, true).map_err(|_| Error)?; - let mut id_hmac = [0; HASH_SIZE]; - Hmac::::mac(&hmac_key, &encrypted_id[..], &mut id_hmac); - encrypted_id.extend(&id_hmac); - Ok(encrypted_id) - } - - #[test] - fn test_encrypt_decrypt_credential_legacy() { - let mut env = TestEnv::default(); - let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::Es256); - let rp_id_hash = [0x55; 32]; - let credential_source = CredentialSource { - private_key, - rp_id_hash, - cred_protect_policy: None, - cred_blob: None, - }; - let ecdsa_key = credential_source.private_key.ecdsa_key(&mut env).unwrap(); - let credential_id = - legacy_encrypt_to_credential_id(&mut env, ecdsa_key, &rp_id_hash).unwrap(); - let unwrapped = env - .key_store() - .unwrap_credential(&credential_id, &rp_id_hash) - .unwrap() - .unwrap(); - assert_eq!(credential_source, unwrapped); - } - #[test] fn test_wrap_credential_size() { let mut env = TestEnv::default(); diff --git a/libraries/opensk/src/api/private_key.rs b/libraries/opensk/src/api/private_key.rs index e9ecf82d..39745e88 100644 --- a/libraries/opensk/src/api/private_key.rs +++ b/libraries/opensk/src/api/private_key.rs @@ -13,11 +13,11 @@ // limitations under the License. use crate::api::crypto::ecdsa::{SecretKey as _, Signature}; -use crate::api::key_store::KeyStore; +use crate::ctap::crypto_wrapper::{aes256_cbc_decrypt, aes256_cbc_encrypt}; use crate::ctap::data_formats::{extract_array, extract_byte_string, CoseKey, SignatureAlgorithm}; use crate::ctap::secret::Secret; use crate::ctap::status_code::Ctap2StatusCode; -use crate::env::{EcdsaSk, Env}; +use crate::env::{AesKey, EcdsaSk, Env}; use alloc::vec; use alloc::vec::Vec; use core::convert::TryFrom; @@ -34,8 +34,7 @@ use sk_cbor::{cbor_array, cbor_bytes, cbor_int}; // We shouldn't compare private keys in prod without constant-time operations. #[cfg_attr(test, derive(PartialEq, Eq))] pub enum PrivateKey { - // 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. + // We store the key bytes instead of the env type. They can be converted into each other. Ecdsa(Secret<[u8; 32]>), #[cfg(feature = "ed25519")] Ed25519(ed25519_compact::SecretKey), @@ -47,10 +46,12 @@ impl PrivateKey { /// # Panics /// /// Panics if the algorithm is [`SignatureAlgorithm::Unknown`]. - pub fn new(env: &mut impl Env, alg: SignatureAlgorithm) -> Self { + pub fn new(env: &mut E, alg: SignatureAlgorithm) -> Self { match alg { SignatureAlgorithm::Es256 => { - PrivateKey::Ecdsa(env.key_store().generate_ecdsa_seed().unwrap()) + let mut bytes: Secret<[u8; 32]> = Secret::default(); + EcdsaSk::::random(env.rng()).to_slice(&mut bytes); + PrivateKey::Ecdsa(bytes) } #[cfg(feature = "ed25519")] SignatureAlgorithm::Eddsa => { @@ -68,8 +69,6 @@ impl PrivateKey { } /// Helper function that creates a private key of type ECDSA. - /// - /// This function is public for legacy credential source parsing only. pub fn new_ecdsa_from_bytes(bytes: &[u8]) -> Option { if bytes.len() != 32 { return None; @@ -79,6 +78,7 @@ impl PrivateKey { Some(PrivateKey::Ecdsa(seed)) } + /// Helper function that creates a private key of type Ed25519. #[cfg(feature = "ed25519")] pub fn new_ed25519_from_bytes(bytes: &[u8]) -> Option { if bytes.len() != 32 { @@ -89,19 +89,19 @@ impl PrivateKey { } /// Returns the ECDSA private key. - pub fn ecdsa_key(&self, env: &mut E) -> Result, Ctap2StatusCode> { + pub fn ecdsa_key(&self) -> Result, Ctap2StatusCode> { match self { - PrivateKey::Ecdsa(seed) => ecdsa_key_from_seed(env, seed), + PrivateKey::Ecdsa(bytes) => ecdsa_key_from_bytes::(bytes), #[allow(unreachable_patterns)] _ => Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR), } } /// Returns the corresponding public key. - pub fn get_pub_key(&self, env: &mut impl Env) -> Result { + pub fn get_pub_key(&self) -> Result { Ok(match self { - PrivateKey::Ecdsa(ecdsa_seed) => { - CoseKey::from_ecdsa_public_key(ecdsa_key_from_seed(env, ecdsa_seed)?.public_key()) + PrivateKey::Ecdsa(bytes) => { + CoseKey::from_ecdsa_public_key(ecdsa_key_from_bytes::(bytes)?.public_key()) } #[cfg(feature = "ed25519")] PrivateKey::Ed25519(ed25519_key) => CoseKey::from(ed25519_key.public_key()), @@ -109,15 +109,9 @@ impl PrivateKey { } /// Returns the encoded signature for a given message. - pub(crate) fn sign_and_encode( - &self, - env: &mut impl Env, - message: &[u8], - ) -> Result, Ctap2StatusCode> { + pub fn sign_and_encode(&self, message: &[u8]) -> Result, Ctap2StatusCode> { Ok(match self { - PrivateKey::Ecdsa(ecdsa_seed) => { - ecdsa_key_from_seed(env, ecdsa_seed)?.sign(message).to_der() - } + PrivateKey::Ecdsa(bytes) => ecdsa_key_from_bytes::(bytes)?.sign(message).to_der(), #[cfg(feature = "ed25519")] PrivateKey::Ed25519(ed25519_key) => ed25519_key.sign(message, None).to_vec(), }) @@ -136,57 +130,55 @@ impl PrivateKey { pub fn to_bytes(&self) -> Secret<[u8]> { let mut bytes = Secret::new(32); match self { - PrivateKey::Ecdsa(ecdsa_seed) => bytes.copy_from_slice(ecdsa_seed.deref()), + PrivateKey::Ecdsa(key_bytes) => bytes.copy_from_slice(key_bytes.deref()), #[cfg(feature = "ed25519")] PrivateKey::Ed25519(ed25519_key) => bytes.copy_from_slice(ed25519_key.seed().deref()), } bytes } -} - -fn ecdsa_key_from_seed( - env: &mut E, - seed: &[u8; 32], -) -> Result, Ctap2StatusCode> { - let ecdsa_bytes = env.key_store().derive_ecdsa(seed)?; - Ok(EcdsaSk::::from_slice(&ecdsa_bytes).unwrap()) -} -impl From<&PrivateKey> for cbor::Value { - /// Writes a private key into CBOR format. This exposes the cryptographic secret. - // TODO needs zeroization if seed is secret - // called in wrap_credential and PublicKeyCredentialSource - fn from(private_key: &PrivateKey) -> Self { - cbor_array![ - cbor_int!(private_key.signature_algorithm() as i64), - cbor_bytes!(private_key.to_bytes().expose_secret_to_vec()), - ] + pub fn to_cbor( + &self, + rng: &mut E::Rng, + wrap_key: &AesKey, + ) -> Result { + let bytes = self.to_bytes(); + let wrapped_bytes = aes256_cbc_encrypt::(rng, wrap_key, &bytes, true)?; + Ok(cbor_array![ + cbor_int!(self.signature_algorithm() as i64), + cbor_bytes!(wrapped_bytes), + ]) } -} - -impl TryFrom for PrivateKey { - type Error = Ctap2StatusCode; - fn try_from(cbor_value: cbor::Value) -> Result { + pub fn from_cbor( + wrap_key: &AesKey, + cbor_value: cbor::Value, + ) -> Result { let mut array = extract_array(cbor_value)?; if array.len() != 2 { return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR); } - let key_bytes = extract_byte_string(array.pop().unwrap())?; + let wrapped_bytes = extract_byte_string(array.pop().unwrap())?; + let key_bytes = aes256_cbc_decrypt::(wrap_key, &wrapped_bytes, true)?; match SignatureAlgorithm::try_from(array.pop().unwrap())? { - SignatureAlgorithm::Es256 => PrivateKey::new_ecdsa_from_bytes(&key_bytes) + SignatureAlgorithm::Es256 => PrivateKey::new_ecdsa_from_bytes(&*key_bytes) .ok_or(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR), #[cfg(feature = "ed25519")] - SignatureAlgorithm::Eddsa => PrivateKey::new_ed25519_from_bytes(&key_bytes) + SignatureAlgorithm::Eddsa => PrivateKey::new_ed25519_from_bytes(&*key_bytes) .ok_or(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR), _ => Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR), } } } +fn ecdsa_key_from_bytes(bytes: &[u8; 32]) -> Result, Ctap2StatusCode> { + EcdsaSk::::from_slice(bytes).ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR) +} + #[cfg(test)] mod test { use super::*; + use crate::api::key_store::KeyStore; use crate::env::test::TestEnv; #[test] @@ -233,10 +225,10 @@ mod test { fn test_private_key_get_pub_key() { let mut env = TestEnv::default(); let private_key = PrivateKey::new_ecdsa(&mut env); - let ecdsa_key = private_key.ecdsa_key(&mut env).unwrap(); + let ecdsa_key = private_key.ecdsa_key::().unwrap(); let public_key = ecdsa_key.public_key(); assert_eq!( - private_key.get_pub_key(&mut env), + private_key.get_pub_key::(), Ok(CoseKey::from_ecdsa_public_key(public_key)) ); } @@ -246,10 +238,10 @@ mod test { let mut env = TestEnv::default(); let message = [0x5A; 32]; let private_key = PrivateKey::new_ecdsa(&mut env); - let ecdsa_key = private_key.ecdsa_key(&mut env).unwrap(); + let ecdsa_key = private_key.ecdsa_key::().unwrap(); let signature = ecdsa_key.sign(&message).to_der(); assert_eq!( - private_key.sign_and_encode(&mut env, &message), + private_key.sign_and_encode::(&message), Ok(signature) ); } @@ -273,9 +265,15 @@ mod test { fn test_private_key_from_to_cbor(signature_algorithm: SignatureAlgorithm) { let mut env = TestEnv::default(); + let wrap_key = env.key_store().wrap_key::().unwrap(); let private_key = PrivateKey::new(&mut env, signature_algorithm); - let cbor = cbor::Value::from(&private_key); - assert_eq!(PrivateKey::try_from(cbor), Ok(private_key),); + let cbor = private_key + .to_cbor::(env.rng(), &wrap_key) + .unwrap(); + assert_eq!( + PrivateKey::from_cbor::(&wrap_key, cbor), + Ok(private_key) + ); } #[test] @@ -290,6 +288,8 @@ mod test { } fn test_private_key_from_bad_cbor(signature_algorithm: SignatureAlgorithm) { + let mut env = TestEnv::default(); + let wrap_key = env.key_store().wrap_key::().unwrap(); let cbor = cbor_array![ cbor_int!(signature_algorithm as i64), cbor_bytes!(vec![0x88; 32]), @@ -297,7 +297,7 @@ mod test { cbor_int!(0), ]; assert_eq!( - PrivateKey::try_from(cbor), + PrivateKey::from_cbor::(&wrap_key, cbor), Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR), ); } @@ -315,13 +315,15 @@ mod test { #[test] fn test_private_key_from_bad_cbor_unsupported_algo() { + let mut env = TestEnv::default(); + let wrap_key = env.key_store().wrap_key::().unwrap(); let cbor = cbor_array![ // This algorithms doesn't exist. cbor_int!(-1), cbor_bytes!(vec![0x88; 32]), ]; assert_eq!( - PrivateKey::try_from(cbor), + PrivateKey::from_cbor::(&wrap_key, cbor), Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR), ); } diff --git a/libraries/opensk/src/ctap/credential_management.rs b/libraries/opensk/src/ctap/credential_management.rs index d546b049..fe0f5ea5 100644 --- a/libraries/opensk/src/ctap/credential_management.rs +++ b/libraries/opensk/src/ctap/credential_management.rs @@ -61,8 +61,7 @@ fn enumerate_rps_response( } /// Generates the response for subcommands enumerating credentials. -fn enumerate_credentials_response( - env: &mut impl Env, +fn enumerate_credentials_response( credential: PublicKeyCredentialSource, total_credentials: Option, ) -> Result { @@ -91,7 +90,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)?; + let public_key = private_key.get_pub_key::()?; Ok(AuthenticatorCredentialManagementResponse { user: Some(user), credential_id: Some(credential_id), @@ -202,7 +201,7 @@ fn process_enumerate_credentials_begin( channel, ); } - enumerate_credentials_response(env, credential, Some(total_credentials as u64)) + enumerate_credentials_response::(credential, Some(total_credentials as u64)) } /// Processes the subcommand enumerateCredentialsGetNextCredential for CredentialManagement. @@ -212,7 +211,7 @@ fn process_enumerate_credentials_get_next_credential( ) -> Result { let credential_key = stateful_command_permission.next_enumerate_credential(env)?; let credential = storage::get_credential(env, credential_key)?; - enumerate_credentials_response(env, credential, None) + enumerate_credentials_response::(credential, None) } /// Processes the subcommand deleteCredential for CredentialManagement. diff --git a/libraries/opensk/src/ctap/crypto_wrapper.rs b/libraries/opensk/src/ctap/crypto_wrapper.rs index f96fda28..01050cee 100644 --- a/libraries/opensk/src/ctap/crypto_wrapper.rs +++ b/libraries/opensk/src/ctap/crypto_wrapper.rs @@ -21,7 +21,7 @@ use rand_core::RngCore; /// Wraps the AES256-CBC encryption to match what we need in CTAP. pub fn aes256_cbc_encrypt( - env: &mut E, + rng: &mut E::Rng, aes_key: &AesKey, plaintext: &[u8], embeds_iv: bool, @@ -32,7 +32,7 @@ pub fn aes256_cbc_encrypt( let mut ciphertext = Vec::with_capacity(plaintext.len() + 16 * embeds_iv as usize); let iv = if embeds_iv { ciphertext.resize(16, 0); - env.rng().fill_bytes(&mut ciphertext[..16]); + rng.fill_bytes(&mut ciphertext[..16]); *array_ref!(ciphertext, 0, 16) } else { [0u8; 16] @@ -74,7 +74,8 @@ mod test { let mut env = TestEnv::default(); let aes_key = AesKey::::new(&[0xC2; 32]); let plaintext = vec![0xAA; 64]; - let ciphertext = aes256_cbc_encrypt(&mut env, &aes_key, &plaintext, true).unwrap(); + let ciphertext = + aes256_cbc_encrypt::(env.rng(), &aes_key, &plaintext, true).unwrap(); let decrypted = aes256_cbc_decrypt::(&aes_key, &ciphertext, true).unwrap(); assert_eq!(*decrypted, plaintext); } @@ -84,7 +85,8 @@ mod test { let mut env = TestEnv::default(); let aes_key = AesKey::::new(&[0xC2; 32]); let plaintext = vec![0xAA; 64]; - let ciphertext = aes256_cbc_encrypt(&mut env, &aes_key, &plaintext, false).unwrap(); + let ciphertext = + aes256_cbc_encrypt::(env.rng(), &aes_key, &plaintext, false).unwrap(); let decrypted = aes256_cbc_decrypt::(&aes_key, &ciphertext, false).unwrap(); assert_eq!(*decrypted, plaintext); } @@ -95,7 +97,7 @@ mod test { let aes_key = AesKey::::new(&[0xC2; 32]); let plaintext = vec![0xAA; 64]; let mut ciphertext_no_iv = - aes256_cbc_encrypt(&mut env, &aes_key, &plaintext, false).unwrap(); + aes256_cbc_encrypt::(env.rng(), &aes_key, &plaintext, false).unwrap(); let mut ciphertext_with_iv = vec![0u8; 16]; ciphertext_with_iv.append(&mut ciphertext_no_iv); let decrypted = aes256_cbc_decrypt::(&aes_key, &ciphertext_with_iv, true).unwrap(); @@ -107,7 +109,8 @@ mod test { let mut env = TestEnv::default(); let aes_key = AesKey::::new(&[0xC2; 32]); let plaintext = vec![0xAA; 64]; - let mut ciphertext = aes256_cbc_encrypt(&mut env, &aes_key, &plaintext, true).unwrap(); + let mut ciphertext = + aes256_cbc_encrypt::(env.rng(), &aes_key, &plaintext, true).unwrap(); let mut expected_plaintext = plaintext; for i in 0..16 { ciphertext[i] ^= 0xBB; @@ -122,8 +125,10 @@ mod test { let mut env = TestEnv::default(); let aes_key = AesKey::::new(&[0xC2; 32]); let plaintext = vec![0xAA; 64]; - let ciphertext1 = aes256_cbc_encrypt(&mut env, &aes_key, &plaintext, true).unwrap(); - let ciphertext2 = aes256_cbc_encrypt(&mut env, &aes_key, &plaintext, true).unwrap(); + let ciphertext1 = + aes256_cbc_encrypt::(env.rng(), &aes_key, &plaintext, true).unwrap(); + let ciphertext2 = + aes256_cbc_encrypt::(env.rng(), &aes_key, &plaintext, true).unwrap(); assert_eq!(ciphertext1.len(), 80); assert_eq!(ciphertext2.len(), 80); // The ciphertext should mutate in all blocks with a different IV. diff --git a/libraries/opensk/src/ctap/ctap1.rs b/libraries/opensk/src/ctap/ctap1.rs index 841df67a..bb9d326e 100644 --- a/libraries/opensk/src/ctap/ctap1.rs +++ b/libraries/opensk/src/ctap/ctap1.rs @@ -256,7 +256,7 @@ impl Ctap1Command { ) -> Result, Ctap1StatusCode> { let private_key = PrivateKey::new_ecdsa(env); let sk = private_key - .ecdsa_key(env) + .ecdsa_key::() .map_err(|_| Ctap1StatusCode::SW_INTERNAL_EXCEPTION)?; let pk = sk.public_key(); let credential_source = CredentialSource { @@ -333,10 +333,6 @@ impl Ctap1Command { .map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?; let credential_source = filter_listed_credential(credential_source, false) .ok_or(Ctap1StatusCode::SW_WRONG_DATA)?; - let ecdsa_key = credential_source - .private_key - .ecdsa_key(env) - .map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?; if flags == Ctap1Flags::CheckOnly { return Err(Ctap1StatusCode::SW_COND_USE_NOT_SATISFIED); } @@ -351,10 +347,13 @@ impl Ctap1Command { ) .map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?; signature_data.extend(&challenge); - let signature = ecdsa_key.sign(&signature_data); + let signature = credential_source + .private_key + .sign_and_encode::(&signature_data) + .map_err(|_| Ctap1StatusCode::SW_INTERNAL_EXCEPTION)?; let mut response = signature_data[application.len()..application.len() + 5].to_vec(); - response.extend(signature.to_der()); + response.extend(&signature); Ok(response) } } diff --git a/libraries/opensk/src/ctap/data_formats.rs b/libraries/opensk/src/ctap/data_formats.rs index d10d2408..c7ccb6d4 100644 --- a/libraries/opensk/src/ctap/data_formats.rs +++ b/libraries/opensk/src/ctap/data_formats.rs @@ -15,6 +15,7 @@ use super::status_code::Ctap2StatusCode; use crate::api::crypto::{ecdh, ecdsa, EC_FIELD_SIZE}; use crate::api::private_key::PrivateKey; +use crate::env::{AesKey, Env}; use alloc::string::String; use alloc::vec::Vec; #[cfg(feature = "fuzz")] @@ -601,8 +602,6 @@ pub struct PublicKeyCredentialSource { // is associated with a unique tag, implemented with a CBOR unsigned key. enum PublicKeyCredentialSourceField { CredentialId = 0, - // Deprecated, we still read this field for backwards compatibility. - EcdsaPrivateKey = 1, RpId = 2, UserHandle = 3, UserDisplayName = 4, @@ -616,6 +615,7 @@ enum PublicKeyCredentialSourceField { // When a field is removed, its tag should be reserved and not used for new fields. We document // those reserved tags below. // Reserved tags: + // - EcdsaPrivateKey = 1, // - CredRandom = 5, } @@ -625,32 +625,41 @@ impl From for cbor::Value { } } -impl From for cbor::Value { - fn from(credential: PublicKeyCredentialSource) -> cbor::Value { - cbor_map_options! { - PublicKeyCredentialSourceField::CredentialId => Some(credential.credential_id), - PublicKeyCredentialSourceField::RpId => Some(credential.rp_id), - PublicKeyCredentialSourceField::UserHandle => Some(credential.user_handle), - PublicKeyCredentialSourceField::UserDisplayName => credential.user_display_name, - PublicKeyCredentialSourceField::CredProtectPolicy => credential.cred_protect_policy, - PublicKeyCredentialSourceField::CreationOrder => credential.creation_order, - PublicKeyCredentialSourceField::UserName => credential.user_name, - PublicKeyCredentialSourceField::UserIcon => credential.user_icon, - PublicKeyCredentialSourceField::CredBlob => credential.cred_blob, - PublicKeyCredentialSourceField::LargeBlobKey => credential.large_blob_key, - PublicKeyCredentialSourceField::PrivateKey => credential.private_key, - } +impl PublicKeyCredentialSource { + // Relying parties do not need to provide the credential ID in an allow_list if true. + pub fn is_discoverable(&self) -> bool { + self.cred_protect_policy.is_none() + || self.cred_protect_policy + == Some(CredentialProtectionPolicy::UserVerificationOptional) } -} -impl TryFrom for PublicKeyCredentialSource { - type Error = Ctap2StatusCode; + pub fn to_cbor( + self, + rng: &mut E::Rng, + wrap_key: &AesKey, + ) -> Result { + Ok(cbor_map_options! { + PublicKeyCredentialSourceField::CredentialId => Some(self.credential_id), + PublicKeyCredentialSourceField::RpId => Some(self.rp_id), + PublicKeyCredentialSourceField::UserHandle => Some(self.user_handle), + PublicKeyCredentialSourceField::UserDisplayName => self.user_display_name, + PublicKeyCredentialSourceField::CredProtectPolicy => self.cred_protect_policy, + PublicKeyCredentialSourceField::CreationOrder => self.creation_order, + PublicKeyCredentialSourceField::UserName => self.user_name, + PublicKeyCredentialSourceField::UserIcon => self.user_icon, + PublicKeyCredentialSourceField::CredBlob => self.cred_blob, + PublicKeyCredentialSourceField::LargeBlobKey => self.large_blob_key, + PublicKeyCredentialSourceField::PrivateKey => self.private_key.to_cbor::(rng, wrap_key)?, + }) + } - fn try_from(cbor_value: cbor::Value) -> Result { + pub fn from_cbor( + wrap_key: &AesKey, + cbor_value: cbor::Value, + ) -> Result { destructure_cbor_map! { let { PublicKeyCredentialSourceField::CredentialId => credential_id, - PublicKeyCredentialSourceField::EcdsaPrivateKey => ecdsa_private_key, PublicKeyCredentialSourceField::RpId => rp_id, PublicKeyCredentialSourceField::UserHandle => user_handle, PublicKeyCredentialSourceField::UserDisplayName => user_display_name, @@ -676,17 +685,7 @@ impl TryFrom for PublicKeyCredentialSource { let user_icon = user_icon.map(extract_text_string).transpose()?; let cred_blob = cred_blob.map(extract_byte_string).transpose()?; let large_blob_key = large_blob_key.map(extract_byte_string).transpose()?; - - // Parse the private key from the deprecated field if necessary. - let ecdsa_private_key = ecdsa_private_key.map(extract_byte_string).transpose()?; - let private_key = private_key.map(PrivateKey::try_from).transpose()?; - let private_key = match (ecdsa_private_key, private_key) { - (None, None) => return Err(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER), - (Some(_), Some(_)) => return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR), - (Some(k), None) => PrivateKey::new_ecdsa_from_bytes(&k) - .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?, - (None, Some(k)) => k, - }; + let private_key = PrivateKey::from_cbor::(wrap_key, ok_or_missing(private_key)?)?; // We don't return whether there were unknown fields in the CBOR value. This means that // deserialization is not injective. In particular deserialization is only an inverse of @@ -715,15 +714,6 @@ impl TryFrom for PublicKeyCredentialSource { } } -impl PublicKeyCredentialSource { - // Relying parties do not need to provide the credential ID in an allow_list if true. - pub fn is_discoverable(&self) -> bool { - self.cred_protect_policy.is_none() - || self.cred_protect_policy - == Some(CredentialProtectionPolicy::UserVerificationOptional) - } -} - // The COSE key is used for both ECDH and ECDSA public keys for transmission. #[derive(Clone, Debug, PartialEq, Eq)] #[cfg_attr(feature = "fuzz", derive(Arbitrary))] @@ -1232,6 +1222,7 @@ mod test { use super::*; use crate::api::crypto::ecdh::PublicKey as _; use crate::api::crypto::ecdsa::PublicKey as _; + use crate::api::key_store::KeyStore; use crate::api::rng::Rng; use crate::env::test::TestEnv; use crate::env::{EcdhPk, EcdsaPk, Env}; @@ -2105,6 +2096,7 @@ mod test { #[test] fn test_credential_source_cbor_round_trip() { let mut env = TestEnv::default(); + let wrap_key = env.key_store().wrap_key::().unwrap(); let private_key = PrivateKey::new_ecdsa(&mut env); let credential = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, @@ -2121,8 +2113,12 @@ mod test { large_blob_key: None, }; + let cbor_value = credential + .clone() + .to_cbor::(env.rng(), &wrap_key) + .unwrap(); assert_eq!( - PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), + PublicKeyCredentialSource::from_cbor::(&wrap_key, cbor_value), Ok(credential.clone()) ); @@ -2131,8 +2127,12 @@ mod test { ..credential }; + let cbor_value = credential + .clone() + .to_cbor::(env.rng(), &wrap_key) + .unwrap(); assert_eq!( - PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), + PublicKeyCredentialSource::from_cbor::(&wrap_key, cbor_value), Ok(credential.clone()) ); @@ -2141,8 +2141,12 @@ mod test { ..credential }; + let cbor_value = credential + .clone() + .to_cbor::(env.rng(), &wrap_key) + .unwrap(); assert_eq!( - PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), + PublicKeyCredentialSource::from_cbor::(&wrap_key, cbor_value), Ok(credential.clone()) ); @@ -2151,8 +2155,12 @@ mod test { ..credential }; + let cbor_value = credential + .clone() + .to_cbor::(env.rng(), &wrap_key) + .unwrap(); assert_eq!( - PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), + PublicKeyCredentialSource::from_cbor::(&wrap_key, cbor_value), Ok(credential.clone()) ); @@ -2161,8 +2169,12 @@ mod test { ..credential }; + let cbor_value = credential + .clone() + .to_cbor::(env.rng(), &wrap_key) + .unwrap(); assert_eq!( - PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), + PublicKeyCredentialSource::from_cbor::(&wrap_key, cbor_value), Ok(credential.clone()) ); @@ -2171,8 +2183,12 @@ mod test { ..credential }; + let cbor_value = credential + .clone() + .to_cbor::(env.rng(), &wrap_key) + .unwrap(); assert_eq!( - PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), + PublicKeyCredentialSource::from_cbor::(&wrap_key, cbor_value), Ok(credential.clone()) ); @@ -2181,91 +2197,28 @@ mod test { ..credential }; + let cbor_value = credential + .clone() + .to_cbor::(env.rng(), &wrap_key) + .unwrap(); assert_eq!( - PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), - Ok(credential) - ); - } - - #[test] - fn test_credential_source_cbor_read_legacy() { - let mut env = TestEnv::default(); - 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, - rp_id: "example.com".to_string(), - user_handle: b"foo".to_vec(), - user_display_name: None, - cred_protect_policy: None, - creation_order: 0, - user_name: None, - user_icon: None, - cred_blob: None, - large_blob_key: None, - }; - - let source_cbor = cbor_map! { - PublicKeyCredentialSourceField::CredentialId => credential.credential_id.clone(), - PublicKeyCredentialSourceField::EcdsaPrivateKey => key_bytes, - PublicKeyCredentialSourceField::RpId => credential.rp_id.clone(), - PublicKeyCredentialSourceField::UserHandle => credential.user_handle.clone(), - }; - assert_eq!( - PublicKeyCredentialSource::try_from(source_cbor), + PublicKeyCredentialSource::from_cbor::(&wrap_key, cbor_value), Ok(credential) ); } #[test] - fn test_credential_source_cbor_legacy_error() { + fn test_credential_source_invalid_cbor() { let mut env = TestEnv::default(); - 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: private_key.clone(), - rp_id: "example.com".to_string(), - user_handle: b"foo".to_vec(), - user_display_name: None, - cred_protect_policy: None, - creation_order: 0, - user_name: None, - user_icon: None, - cred_blob: None, - large_blob_key: None, - }; - - let source_cbor = cbor_map! { - PublicKeyCredentialSourceField::CredentialId => credential.credential_id.clone(), - PublicKeyCredentialSourceField::RpId => credential.rp_id.clone(), - PublicKeyCredentialSourceField::UserHandle => credential.user_handle.clone(), - }; - assert_eq!( - PublicKeyCredentialSource::try_from(source_cbor), - Err(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER) - ); - - let source_cbor = cbor_map! { - PublicKeyCredentialSourceField::CredentialId => credential.credential_id, - PublicKeyCredentialSourceField::EcdsaPrivateKey => key_bytes, - PublicKeyCredentialSourceField::RpId => credential.rp_id, - PublicKeyCredentialSourceField::UserHandle => credential.user_handle, - PublicKeyCredentialSourceField::PrivateKey => private_key, - }; - assert_eq!( - PublicKeyCredentialSource::try_from(source_cbor), - Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR) + let wrap_key = env.key_store().wrap_key::().unwrap(); + assert!(PublicKeyCredentialSource::from_cbor::(&wrap_key, cbor_false!()).is_err()); + assert!( + PublicKeyCredentialSource::from_cbor::(&wrap_key, cbor_array!(false)).is_err() ); - } - - #[test] - fn test_credential_source_invalid_cbor() { - assert!(PublicKeyCredentialSource::try_from(cbor_false!()).is_err()); - assert!(PublicKeyCredentialSource::try_from(cbor_array!(false)).is_err()); - assert!(PublicKeyCredentialSource::try_from(cbor_array!(b"foo".to_vec())).is_err()); + assert!(PublicKeyCredentialSource::from_cbor::( + &wrap_key, + cbor_array!(b"foo".to_vec()) + ) + .is_err()); } } diff --git a/libraries/opensk/src/ctap/mod.rs b/libraries/opensk/src/ctap/mod.rs index 77285108..5c5d1eda 100644 --- a/libraries/opensk/src/ctap/mod.rs +++ b/libraries/opensk/src/ctap/mod.rs @@ -917,7 +917,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)?; + let public_cose_key = private_key.get_pub_key::()?; cbor_write(cbor::Value::from(public_cose_key), &mut auth_data)?; if has_extension_output { let hmac_secret_output = if extensions.hmac_secret { @@ -965,7 +965,7 @@ impl CtapState { Some(vec![certificate]), ) } - None => (private_key.sign_and_encode(env, &signature_data)?, None), + None => (private_key.sign_and_encode::(&signature_data)?, None), }; let attestation_statement = PackedAttestationStatement { alg: SignatureAlgorithm::Es256 as i64, @@ -1051,7 +1051,7 @@ impl CtapState { signature_data.extend(client_data_hash); let signature = credential .private_key - .sign_and_encode(env, &signature_data)?; + .sign_and_encode::(&signature_data)?; let cred_desc = PublicKeyCredentialDescriptor { key_type: PublicKeyCredentialType::PublicKey, diff --git a/libraries/opensk/src/ctap/pin_protocol.rs b/libraries/opensk/src/ctap/pin_protocol.rs index 4048bcd2..dc3a5a88 100644 --- a/libraries/opensk/src/ctap/pin_protocol.rs +++ b/libraries/opensk/src/ctap/pin_protocol.rs @@ -225,7 +225,7 @@ impl SharedSecretV1 { } fn encrypt(&self, env: &mut E, plaintext: &[u8]) -> Result, Ctap2StatusCode> { - aes256_cbc_encrypt(env, &self.aes_key, plaintext, false) + aes256_cbc_encrypt::(env.rng(), &self.aes_key, plaintext, false) } fn decrypt(&self, ciphertext: &[u8]) -> Result, Ctap2StatusCode> { @@ -263,7 +263,7 @@ impl SharedSecretV2 { } fn encrypt(&self, env: &mut E, plaintext: &[u8]) -> Result, Ctap2StatusCode> { - aes256_cbc_encrypt(env, &self.aes_key, plaintext, true) + aes256_cbc_encrypt::(env.rng(), &self.aes_key, plaintext, true) } fn decrypt(&self, ciphertext: &[u8]) -> Result, Ctap2StatusCode> { diff --git a/libraries/opensk/src/ctap/storage.rs b/libraries/opensk/src/ctap/storage.rs index aabc0e27..bcf27123 100644 --- a/libraries/opensk/src/ctap/storage.rs +++ b/libraries/opensk/src/ctap/storage.rs @@ -24,13 +24,12 @@ use crate::ctap::data_formats::{ }; use crate::ctap::status_code::Ctap2StatusCode; use crate::ctap::INITIAL_SIGNATURE_COUNTER; -use crate::env::Env; +use crate::env::{AesKey, Env}; use alloc::string::String; use alloc::vec; use alloc::vec::Vec; use arrayref::array_ref; use core::cmp; -use core::convert::TryInto; use persistent_store::{fragment, StoreUpdate}; #[cfg(feature = "config_command")] use sk_cbor::cbor_array_vec; @@ -56,8 +55,8 @@ pub fn init(env: &mut impl Env) -> Result<(), Ctap2StatusCode> { /// # Errors /// /// Returns `CTAP2_ERR_VENDOR_INTERNAL_ERROR` if the key does not hold a valid credential. -pub fn get_credential( - env: &mut impl Env, +pub fn get_credential( + env: &mut E, key: usize, ) -> Result { let min_key = key::CREDENTIALS.start; @@ -68,7 +67,8 @@ pub fn get_credential( .store() .find(key)? .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?; - deserialize_credential(&credential_entry) + let wrap_key = env.key_store().wrap_key::()?; + deserialize_credential::(&wrap_key, &credential_entry) .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR) } @@ -118,8 +118,8 @@ pub fn find_credential( /// Stores or updates a credential. /// /// If a credential with the same RP id and user handle already exists, it is replaced. -pub fn store_credential( - env: &mut impl Env, +pub fn store_credential( + env: &mut E, new_credential: PublicKeyCredentialSource, ) -> Result<(), Ctap2StatusCode> { let max_supported_resident_keys = env.customization().max_supported_resident_keys(); @@ -158,7 +158,8 @@ pub fn store_credential( // This is an existing credential being updated, we reuse its key. Some(x) => x, }; - let value = serialize_credential(new_credential)?; + let wrap_key = env.key_store().wrap_key::()?; + let value = serialize_credential::(env, &wrap_key, new_credential)?; env.store().insert(key, &value)?; Ok(()) } @@ -178,8 +179,8 @@ pub fn delete_credential(env: &mut impl Env, credential_id: &[u8]) -> Result<(), /// # Errors /// /// Returns `CTAP2_ERR_NO_CREDENTIALS` if the credential is not found. -pub fn update_credential( - env: &mut impl Env, +pub fn update_credential( + env: &mut E, credential_id: &[u8], user: PublicKeyCredentialUserEntity, ) -> Result<(), Ctap2StatusCode> { @@ -187,7 +188,8 @@ pub fn update_credential( credential.user_name = user.user_name; credential.user_display_name = user.user_display_name; credential.user_icon = user.user_icon; - let value = serialize_credential(credential)?; + let wrap_key = env.key_store().wrap_key::()?; + let value = serialize_credential::(env, &wrap_key, credential)?; Ok(env.store().insert(key, &value)?) } @@ -215,7 +217,7 @@ pub fn iter_credentials<'a, E: Env>( env: &'a mut E, result: &'a mut Result<(), Ctap2StatusCode>, ) -> Result, Ctap2StatusCode> { - IterCredentials::new(env.store(), result) + IterCredentials::new(env, result) } /// Returns the next creation order. @@ -525,6 +527,9 @@ pub struct IterCredentials<'a, E: Env> { /// The store being iterated. store: &'a persistent_store::Store, + /// The key store for credential unwrapping. + wrap_key: AesKey, + /// The store iterator. iter: persistent_store::StoreIter<'a>, @@ -538,12 +543,15 @@ pub struct IterCredentials<'a, E: Env> { impl<'a, E: Env> IterCredentials<'a, E> { /// Creates a credential iterator. fn new( - store: &'a persistent_store::Store, + env: &'a mut E, result: &'a mut Result<(), Ctap2StatusCode>, ) -> Result { + let wrap_key = env.key_store().wrap_key::()?; + let store = env.store(); let iter = store.iter()?; Ok(IterCredentials { store, + wrap_key, iter, result, }) @@ -576,7 +584,8 @@ impl<'a, E: Env> Iterator for IterCredentials<'a, E> { continue; } let value = self.unwrap(handle.get_value(self.store).ok())?; - let credential = self.unwrap(deserialize_credential(&value))?; + let deserialized = deserialize_credential::(&self.wrap_key, &value); + let credential = self.unwrap(deserialized)?; return Some((key, credential)); } None @@ -584,15 +593,22 @@ impl<'a, E: Env> Iterator for IterCredentials<'a, E> { } /// Deserializes a credential from storage representation. -fn deserialize_credential(data: &[u8]) -> Option { +fn deserialize_credential( + wrap_key: &AesKey, + data: &[u8], +) -> Option { let cbor = super::cbor_read(data).ok()?; - cbor.try_into().ok() + PublicKeyCredentialSource::from_cbor::(wrap_key, cbor).ok() } /// Serializes a credential to storage representation. -fn serialize_credential(credential: PublicKeyCredentialSource) -> Result, Ctap2StatusCode> { +fn serialize_credential( + env: &mut E, + wrap_key: &AesKey, + credential: PublicKeyCredentialSource, +) -> Result, Ctap2StatusCode> { let mut data = Vec::new(); - super::cbor_write(credential.into(), &mut data)?; + super::cbor_write(credential.to_cbor::(env.rng(), wrap_key)?, &mut data)?; Ok(data) } @@ -1098,6 +1114,7 @@ mod test { #[test] fn test_serialize_deserialize_credential() { let mut env = TestEnv::default(); + let wrap_key = env.key_store().wrap_key::().unwrap(); let private_key = PrivateKey::new_ecdsa(&mut env); let credential = PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, @@ -1113,8 +1130,9 @@ mod test { cred_blob: Some(vec![0xCB]), large_blob_key: Some(vec![0x1B]), }; - let serialized = serialize_credential(credential.clone()).unwrap(); - let reconstructed = deserialize_credential(&serialized).unwrap(); + let serialized = + serialize_credential::(&mut env, &wrap_key, credential.clone()).unwrap(); + let reconstructed = deserialize_credential::(&wrap_key, &serialized).unwrap(); assert_eq!(credential, reconstructed); }