Merge branch 'develop' into cbor-api

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.

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.

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.

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<SecKey> {
+        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 {

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()

libraries/opensk/src/api/crypto/ecdsa.rs

@@ -73,7 +73,6 @@ pub trait Signature: Sized {
     fn from_slice(bytes: &[u8; EC_SIGNATURE_SIZE]) -> Option<Self>;
 
     /// 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.

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);
     }

libraries/opensk/src/api/key_store.rs

@@ -13,15 +13,14 @@
 // 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;
 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<E: Env>(&mut self) -> Result<AesKey<E>, 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<Secret<[u8; 16]>, Error>;
 
-    /// Derives an ECDSA private key from a seed.
-    ///
-    /// The result is big-endian.
-    fn derive_ecdsa(&mut self, seed: &[u8; 32]) -> Result<Secret<[u8; 32]>, Error>;
-
-    /// Generates a seed to derive an ECDSA private key.
-    fn generate_ecdsa_seed(&mut self) -> Result<Secret<[u8; 32]>, Error>;
-
     /// Resets the key store.
     fn reset(&mut self) -> Result<(), Error>;
 }
@@ -138,14 +136,23 @@ impl<T: Helper> KeyStore for T {
         Ok(())
     }
 
+    fn wrap_key<E: Env>(&mut self) -> Result<AesKey<E>, Error> {
+        Ok(AesKey::<E>::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<Vec<u8>, Error> {
         let mut payload = Vec::new();
+        let wrap_key = self.wrap_key::<T>()?;
+        let private_key_cbor = credential
+            .private_key
+            .to_cbor::<T>(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<T: Helper> KeyStore for T {
         let master_keys = get_master_keys(self)?;
         let aes_key = AesKey::<T>::new(&master_keys.encryption);
         let encrypted_payload =
-            aes256_cbc_encrypt(self, &aes_key, &payload, true).map_err(|_| Error)?;
+            aes256_cbc_encrypt::<T>(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<T: Helper> 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<T: Helper> KeyStore for T {
             return Ok(None);
         }
 
-        let credential_source = if bytes.len() == LEGACY_CREDENTIAL_ID_SIZE {
-            decrypt_legacy_credential_id::<T>(&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::<T>(
-                        &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::<T>(
+                    self,
+                    &master_keys.encryption,
+                    &bytes[1..hmac_message_size],
+                )?
             }
+            _ => return Ok(None),
         };
 
         if let Some(credential_source) = &credential_source {
@@ -241,24 +239,8 @@ impl<T: Helper> KeyStore for T {
         Ok(Secret::from_exposed_secret(*cipher))
     }
 
-    fn derive_ecdsa(&mut self, seed: &[u8; 32]) -> Result<Secret<[u8; 32]>, Error> {
-        match EcdsaSk::<T>::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<Secret<[u8; 32]>, Error> {
-        let mut seed = Secret::default();
-        EcdsaSk::<T>::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<E: Env>(
-    encryption_key_bytes: &[u8; 32],
-    bytes: &[u8],
-) -> Result<Option<CredentialSource>, Error> {
-    let aes_key = AesKey::<E>::new(encryption_key_bytes);
-    let plaintext = aes256_cbc_decrypt::<E>(&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<E: Env>(
+    env: &mut E,
     encryption_key_bytes: &[u8; 32],
     bytes: &[u8],
 ) -> Result<Option<CredentialSource>, Error> {
@@ -376,7 +335,9 @@ fn decrypt_cbor_credential_id<E: Env>(
     }
     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::<E>()?;
+            let private_key =
+                PrivateKey::from_cbor::<E>(&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);
@@ -414,9 +375,11 @@ fn extract_map(cbor_value: cbor::Value) -> Result<Vec<(cbor::Value, cbor::Value)
 #[cfg(test)]
 mod test {
     use super::*;
+    use crate::api::crypto::ecdsa::SecretKey;
     use crate::api::customization::Customization;
     use crate::ctap::data_formats::SignatureAlgorithm;
     use crate::env::test::TestEnv;
+    use crate::env::EcdsaSk;
 
     const UNSUPPORTED_CREDENTIAL_ID_VERSION: u8 = 0x80;
 
@@ -431,16 +394,24 @@ mod test {
         assert_eq!(&key_store.cred_random(false).unwrap(), &cred_random_no_uv);
         assert_eq!(&key_store.cred_random(true).unwrap(), &cred_random_with_uv);
 
-        // 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));
+        // Same for wrap key.
+        let wrap_key = key_store.wrap_key::<TestEnv>().unwrap();
+        let mut test_block = [0x33; 16];
+        wrap_key.encrypt_block(&mut test_block);
+        let new_wrap_key = key_store.wrap_key::<TestEnv>().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::<TestEnv>().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]
@@ -579,49 +550,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<TestEnv>,
-        application: &[u8; 32],
-    ) -> Result<Vec<u8>, Error> {
-        let master_keys = get_master_keys(env).unwrap();
-        let aes_key = AesKey::<TestEnv>::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::<TestEnv>::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();