Make private keys extensible (#476)

* adds PrivateKey to handle different algorithms

* fixes input check problem of decrypt_credential_source

* addresses comments

* version number not encrypted

* version number test

* adds a credential size test

* removes the algorithm from credential encoding

src/ctap/credential_management.rs

@@ -16,7 +16,7 @@ use super::super::clock::CtapInstant;
 use super::client_pin::{ClientPin, PinPermission};
 use super::command::AuthenticatorCredentialManagementParameters;
 use super::data_formats::{
-    CoseKey, CredentialManagementSubCommand, CredentialManagementSubCommandParameters,
+    CredentialManagementSubCommand, CredentialManagementSubCommandParameters,
     PublicKeyCredentialDescriptor, PublicKeyCredentialRpEntity, PublicKeyCredentialSource,
     PublicKeyCredentialUserEntity,
 };
@@ -92,7 +92,7 @@ fn enumerate_credentials_response(
         key_id: credential_id,
         transports: None, // You can set USB as a hint here.
     };
-    let public_key = CoseKey::from(private_key.genpk());
+    let public_key = private_key.get_pub_key();
     Ok(AuthenticatorCredentialManagementResponse {
         user: Some(user),
         credential_id: Some(credential_id),
@@ -359,6 +359,7 @@ pub fn process_credential_management(
 
 #[cfg(test)]
 mod test {
+    use super::super::crypto_wrapper::PrivateKey;
     use super::super::data_formats::{PinUvAuthProtocol, PublicKeyCredentialType};
     use super::super::pin_protocol::authenticate_pin_uv_auth_token;
     use super::super::CtapState;
@@ -373,7 +374,7 @@ mod test {
         PublicKeyCredentialSource {
             key_type: PublicKeyCredentialType::PublicKey,
             credential_id: rng.gen_uniform_u8x32().to_vec(),
-            private_key,
+            private_key: PrivateKey::from(private_key),
             rp_id: String::from("example.com"),
             user_handle: vec![0x01],
             user_display_name: Some("display_name".to_string()),

src/ctap/crypto_wrapper.rs

@@ -12,10 +12,35 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use crate::ctap::data_formats::{
+    extract_array, extract_byte_string, CoseKey, PublicKeyCredentialSource,
+    PublicKeyCredentialType, SignatureAlgorithm,
+};
 use crate::ctap::status_code::Ctap2StatusCode;
+use crate::ctap::storage;
+use crate::env::Env;
+use alloc::string::String;
+use alloc::vec;
 use alloc::vec::Vec;
+use core::convert::TryFrom;
 use crypto::cbc::{cbc_decrypt, cbc_encrypt};
+use crypto::ecdsa;
+use crypto::hmac::{hmac_256, verify_hmac_256};
+use crypto::sha256::Sha256;
 use rng256::Rng256;
+use sk_cbor as cbor;
+use sk_cbor::{cbor_array, cbor_bytes, cbor_int};
+
+// Legacy credential IDs consist of
+// - 16 bytes: initialization vector for AES-256,
+// - 32 bytes: ECDSA private key for the credential,
+// - 32 bytes: relying party ID hashed with SHA256,
+// - 32 bytes: HMAC-SHA256 over everything else.
+pub const LEGACY_CREDENTIAL_ID_SIZE: usize = 112;
+#[cfg(test)]
+pub const ECDSA_CREDENTIAL_ID_SIZE: usize = 113;
+// See encrypt_key_handle v1 documentation.
+pub const MAX_CREDENTIAL_ID_SIZE: usize = 113;
 
 /// Wraps the AES256-CBC encryption to match what we need in CTAP.
 pub fn aes256_cbc_encrypt(
@@ -27,7 +52,8 @@ pub fn aes256_cbc_encrypt(
     if plaintext.len() % 16 != 0 {
         return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER);
     }
-    let mut ciphertext = Vec::with_capacity(plaintext.len() + 16 * embeds_iv as usize);
+    // The extra 1 capacity is because encrypt_key_handle adds a version number.
+    let mut ciphertext = Vec::with_capacity(plaintext.len() + 16 * embeds_iv as usize + 1);
     let iv = if embeds_iv {
         let random_bytes = rng.gen_uniform_u8x32();
         ciphertext.extend_from_slice(&random_bytes[..16]);
@@ -62,6 +88,205 @@ pub fn aes256_cbc_decrypt(
     Ok(plaintext)
 }
 
+/// An asymmetric private key that can sign messages.
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum PrivateKey {
+    Ecdsa(ecdsa::SecKey),
+}
+
+impl PrivateKey {
+    /// Creates a new private key for the given algorithm.
+    ///
+    /// # Panics
+    ///
+    /// Panics if the algorithm is [`SignatureAlgorithm::Unknown`].
+    pub fn new(rng: &mut impl Rng256, alg: SignatureAlgorithm) -> Self {
+        match alg {
+            SignatureAlgorithm::ES256 => PrivateKey::Ecdsa(crypto::ecdsa::SecKey::gensk(rng)),
+            SignatureAlgorithm::Unknown => unreachable!(),
+        }
+    }
+
+    /// 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<Self> {
+        if bytes.len() != 32 {
+            return None;
+        }
+        ecdsa::SecKey::from_bytes(array_ref!(bytes, 0, 32)).map(PrivateKey::from)
+    }
+
+    /// Returns the corresponding public key.
+    pub fn get_pub_key(&self) -> CoseKey {
+        match self {
+            PrivateKey::Ecdsa(ecdsa_key) => CoseKey::from(ecdsa_key.genpk()),
+        }
+    }
+
+    /// Returns the encoded signature for a given message.
+    pub fn sign_and_encode(&self, message: &[u8]) -> Vec<u8> {
+        match self {
+            PrivateKey::Ecdsa(ecdsa_key) => ecdsa_key.sign_rfc6979::<Sha256>(message).to_asn1_der(),
+        }
+    }
+
+    /// The associated COSE signature algorithm identifier.
+    pub fn signature_algorithm(&self) -> SignatureAlgorithm {
+        match self {
+            PrivateKey::Ecdsa(_) => SignatureAlgorithm::ES256,
+        }
+    }
+
+    /// Writes the key bytes.
+    pub fn to_bytes(&self) -> Vec<u8> {
+        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
+            }
+        }
+    }
+}
+
+impl From<PrivateKey> for cbor::Value {
+    fn from(private_key: PrivateKey) -> Self {
+        cbor_array![
+            cbor_int!(private_key.signature_algorithm() as i64),
+            cbor_bytes!(private_key.to_bytes()),
+        ]
+    }
+}
+
+impl TryFrom<cbor::Value> for PrivateKey {
+    type Error = Ctap2StatusCode;
+
+    fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
+        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())?;
+        match SignatureAlgorithm::try_from(array.pop().unwrap())? {
+            SignatureAlgorithm::ES256 => PrivateKey::new_ecdsa_from_bytes(&key_bytes)
+                .ok_or(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR),
+            _ => Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR),
+        }
+    }
+}
+
+impl From<ecdsa::SecKey> 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
+/// stored server-side, this information is already available (unencrypted).
+///
+/// Also, by limiting ourselves to private key and RP ID hash, we are compatible with U2F for
+/// ECDSA private keys.
+///
+/// This is v1, and we write the following data for ECDSA (algorithm -7):
+/// -  1 byte : version number
+/// - 16 bytes: initialization vector for AES-256,
+/// - 32 bytes: ECDSA private key for the credential,
+/// - 32 bytes: relying party ID hashed with SHA256,
+/// - 32 bytes: HMAC-SHA256 over everything else.
+pub fn encrypt_key_handle(
+    env: &mut impl Env,
+    private_key: &PrivateKey,
+    application: &[u8; 32],
+) -> Result<Vec<u8>, Ctap2StatusCode> {
+    let master_keys = storage::master_keys(env)?;
+    let aes_enc_key = crypto::aes256::EncryptionKey::new(&master_keys.encryption);
+
+    let mut encrypted_id = match private_key {
+        PrivateKey::Ecdsa(ecdsa_key) => {
+            let mut plaintext = [0; 64];
+            ecdsa_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)?;
+            // Version number
+            encrypted_id.insert(0, 0x01);
+            encrypted_id
+        }
+    };
+
+    let id_hmac = hmac_256::<Sha256>(&master_keys.hmac, &encrypted_id[..]);
+    encrypted_id.extend(&id_hmac);
+    Ok(encrypted_id)
+}
+
+/// Decrypts a credential ID and writes the private key into a PublicKeyCredentialSource.
+///
+/// Returns None if
+/// - the format does not match any known versions,
+/// - the HMAC test fails or
+/// - the relying party does not match the decrypted relying party ID hash.
+///
+/// This functions reads:
+/// - legacy credentials (no version number),
+/// - v1 (ECDSA)
+pub fn decrypt_credential_source(
+    env: &mut impl Env,
+    credential_id: Vec<u8>,
+    rp_id_hash: &[u8],
+) -> Result<Option<PublicKeyCredentialSource>, Ctap2StatusCode> {
+    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::<Sha256>(
+        &master_keys.hmac,
+        &credential_id[..hmac_message_size],
+        array_ref![credential_id, hmac_message_size, 32],
+    ) {
+        return Ok(None);
+    }
+
+    let payload = if credential_id.len() == LEGACY_CREDENTIAL_ID_SIZE {
+        &credential_id[..hmac_message_size]
+    } else {
+        // Version number check
+        if credential_id[0] != 1 {
+            return Ok(None);
+        }
+        &credential_id[1..hmac_message_size]
+    };
+    if payload.len() != 80 {
+        // We shouldn't have HMAC'ed anything of different length. The check is cheap though.
+        return Ok(None);
+    }
+
+    let aes_enc_key = crypto::aes256::EncryptionKey::new(&master_keys.encryption);
+    let decrypted_id = aes256_cbc_decrypt(&aes_enc_key, payload, true)?;
+
+    if rp_id_hash != &decrypted_id[32..] {
+        return Ok(None);
+    }
+    let sk_option = PrivateKey::new_ecdsa_from_bytes(&decrypted_id[..32]);
+
+    Ok(sk_option.map(|sk| PublicKeyCredentialSource {
+        key_type: PublicKeyCredentialType::PublicKey,
+        credential_id,
+        private_key: sk,
+        rp_id: String::from(""),
+        user_handle: 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,
+    }))
+}
+
 #[cfg(test)]
 mod test {
     use super::*;
@@ -131,4 +356,199 @@ mod test {
             assert_ne!(block1, block2);
         }
     }
+
+    #[test]
+    fn test_new_ecdsa_from_bytes() {
+        let mut env = TestEnv::new();
+        let private_key = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
+        let key_bytes = private_key.to_bytes();
+        assert_eq!(
+            PrivateKey::new_ecdsa_from_bytes(&key_bytes),
+            Some(private_key)
+        );
+    }
+
+    #[test]
+    fn test_new_ecdsa_from_bytes_wrong_length() {
+        assert_eq!(PrivateKey::new_ecdsa_from_bytes(&[0x55; 16]), None);
+        assert_eq!(PrivateKey::new_ecdsa_from_bytes(&[0x55; 31]), None);
+        assert_eq!(PrivateKey::new_ecdsa_from_bytes(&[0x55; 33]), None);
+        assert_eq!(PrivateKey::new_ecdsa_from_bytes(&[0x55; 64]), None);
+    }
+
+    #[test]
+    fn test_private_key_get_pub_key() {
+        let mut env = TestEnv::new();
+        let ecdsa_key = crypto::ecdsa::SecKey::gensk(env.rng());
+        let public_key = ecdsa_key.genpk();
+        let private_key = PrivateKey::from(ecdsa_key);
+        assert_eq!(private_key.get_pub_key(), 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 signature = ecdsa_key.sign_rfc6979::<Sha256>(&message).to_asn1_der();
+        let private_key = PrivateKey::from(ecdsa_key);
+        assert_eq!(private_key.sign_and_encode(&message), signature);
+    }
+
+    #[test]
+    fn test_private_key_signature_algorithm() {
+        let mut env = TestEnv::new();
+        let algorithm = SignatureAlgorithm::ES256;
+        let private_key = PrivateKey::new(env.rng(), algorithm);
+        assert_eq!(private_key.signature_algorithm(), algorithm);
+    }
+
+    #[test]
+    fn test_private_key_from_to_cbor() {
+        let mut env = TestEnv::new();
+        let private_key = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
+        let cbor = cbor::Value::from(private_key.clone());
+        assert_eq!(PrivateKey::try_from(cbor), Ok(private_key),);
+    }
+
+    #[test]
+    fn test_private_key_from_bad_cbor() {
+        let cbor = cbor_array![
+            cbor_int!(SignatureAlgorithm::ES256 as i64),
+            cbor_bytes!(vec![0x88; 32]),
+            // The array is too long.
+            cbor_int!(0),
+        ];
+        assert_eq!(
+            PrivateKey::try_from(cbor),
+            Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR),
+        );
+
+        let cbor = cbor_array![
+            // This algorithms doesn't exist.
+            cbor_int!(-1),
+            cbor_bytes!(vec![0x88; 32]),
+        ];
+        assert_eq!(
+            PrivateKey::try_from(cbor),
+            Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR),
+        );
+    }
+
+    #[test]
+    fn test_encrypt_decrypt_credential() {
+        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];
+        let encrypted_id = encrypt_key_handle(&mut env, &private_key, &rp_id_hash).unwrap();
+        let decrypted_source = decrypt_credential_source(&mut env, encrypted_id, &rp_id_hash)
+            .unwrap()
+            .unwrap();
+
+        assert_eq!(private_key, decrypted_source.private_key);
+    }
+
+    #[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];
+        let mut encrypted_id = encrypt_key_handle(&mut env, &private_key, &rp_id_hash).unwrap();
+        // Version 2 does not exist yet.
+        encrypted_id[0] = 0x02;
+        // 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::<Sha256>(&master_keys.hmac, &encrypted_id[..]);
+        encrypted_id.extend(&id_hmac);
+
+        assert_eq!(
+            decrypt_credential_source(&mut env, encrypted_id, &rp_id_hash),
+            Ok(None)
+        );
+    }
+
+    #[test]
+    fn test_encrypt_decrypt_bad_hmac() {
+        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];
+        let encrypted_id = encrypt_key_handle(&mut env, &private_key, &rp_id_hash).unwrap();
+        for i in 0..encrypted_id.len() {
+            let mut modified_id = encrypted_id.clone();
+            modified_id[i] ^= 0x01;
+            assert_eq!(
+                decrypt_credential_source(&mut env, modified_id, &rp_id_hash),
+                Ok(None)
+            );
+        }
+    }
+
+    #[test]
+    fn test_decrypt_credential_missing_blocks() {
+        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];
+        let encrypted_id = encrypt_key_handle(&mut env, &private_key, &rp_id_hash).unwrap();
+
+        for length in (1..ECDSA_CREDENTIAL_ID_SIZE).step_by(16) {
+            assert_eq!(
+                decrypt_credential_source(&mut env, encrypted_id[..length].to_vec(), &rp_id_hash),
+                Ok(None)
+            );
+        }
+    }
+
+    /// This is a copy of the function that genereated deprecated key handles.
+    fn legacy_encrypt_key_handle(
+        env: &mut impl Env,
+        private_key: crypto::ecdsa::SecKey,
+        application: &[u8; 32],
+    ) -> Result<Vec<u8>, Ctap2StatusCode> {
+        let master_keys = storage::master_keys(env)?;
+        let aes_enc_key = crypto::aes256::EncryptionKey::new(&master_keys.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::<Sha256>(&master_keys.hmac, &encrypted_id[..]);
+        encrypted_id.extend(&id_hmac);
+        Ok(encrypted_id)
+    }
+
+    #[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());
+
+        let rp_id_hash = [0x55; 32];
+        let encrypted_id = legacy_encrypt_key_handle(&mut env, ecdsa_key, &rp_id_hash).unwrap();
+        let decrypted_source = decrypt_credential_source(&mut env, encrypted_id, &rp_id_hash)
+            .unwrap()
+            .unwrap();
+
+        assert_eq!(private_key, decrypted_source.private_key);
+    }
+
+    #[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];
+        let encrypted_id = encrypt_key_handle(&mut env, &private_key, &rp_id_hash).unwrap();
+        assert_eq!(encrypted_id.len(), ECDSA_CREDENTIAL_ID_SIZE);
+    }
 }

src/ctap/ctap1.rs

@@ -14,6 +14,7 @@
 
 use super::super::clock::CtapInstant;
 use super::apdu::{Apdu, ApduStatusCode};
+use super::crypto_wrapper::{decrypt_credential_source, encrypt_key_handle, PrivateKey};
 use super::CtapState;
 use crate::ctap::storage;
 use crate::env::Env;
@@ -195,7 +196,7 @@ impl Ctap1Command {
                 if !ctap_state.u2f_up_state.consume_up(clock_value) {
                     return Err(Ctap1StatusCode::SW_COND_USE_NOT_SATISFIED);
                 }
-                Ctap1Command::process_register(env, challenge, application, ctap_state)
+                Ctap1Command::process_register(env, challenge, application)
             }
 
             U2fCommand::Authenticate {
@@ -243,12 +244,10 @@ impl Ctap1Command {
         env: &mut impl Env,
         challenge: [u8; 32],
         application: [u8; 32],
-        ctap_state: &mut CtapState,
     ) -> Result<Vec<u8>, Ctap1StatusCode> {
         let sk = crypto::ecdsa::SecKey::gensk(env.rng());
         let pk = sk.genpk();
-        let key_handle = ctap_state
+        let key_handle = encrypt_key_handle(env, &PrivateKey::from(sk), &application)
-            .encrypt_key_handle(env, sk, &application)
             .map_err(|_| Ctap1StatusCode::SW_INTERNAL_EXCEPTION)?;
         if key_handle.len() > 0xFF {
             // This is just being defensive with unreachable code.
@@ -307,10 +306,15 @@ impl Ctap1Command {
         flags: Ctap1Flags,
         ctap_state: &mut CtapState,
     ) -> Result<Vec<u8>, Ctap1StatusCode> {
-        let credential_source = ctap_state
+        let credential_source = decrypt_credential_source(env, key_handle, &application)
-            .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),
+            };
             if flags == Ctap1Flags::CheckOnly {
                 return Err(Ctap1StatusCode::SW_COND_USE_NOT_SATISFIED);
             }
@@ -325,9 +329,7 @@ impl Ctap1Command {
                 )
                 .map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?;
             signature_data.extend(&challenge);
-            let signature = credential_source
+            let signature = ecdsa_key.sign_rfc6979::<crypto::sha256::Sha256>(&signature_data);
-                .private_key
-                .sign_rfc6979::<crypto::sha256::Sha256>(&signature_data);
 
             let mut response = signature_data[application.len()..application.len() + 5].to_vec();
             response.extend(signature.to_asn1_der());
@@ -340,7 +342,9 @@ impl Ctap1Command {
 
 #[cfg(test)]
 mod test {
-    use super::super::{key_material, CREDENTIAL_ID_SIZE};
+    use super::super::crypto_wrapper::ECDSA_CREDENTIAL_ID_SIZE;
+    use super::super::data_formats::SignatureAlgorithm;
+    use super::super::key_material;
     use super::*;
     use crate::api::customization::Customization;
     use crate::clock::TEST_CLOCK_FREQUENCY_HZ;
@@ -375,12 +379,12 @@ mod test {
             0x00,
             0x00,
             0x00,
-            65 + CREDENTIAL_ID_SIZE as u8,
+            65 + ECDSA_CREDENTIAL_ID_SIZE as u8,
         ];
         let challenge = [0x0C; 32];
         message.extend(&challenge);
         message.extend(application);
-        message.push(CREDENTIAL_ID_SIZE as u8);
+        message.push(ECDSA_CREDENTIAL_ID_SIZE as u8);
         message.extend(key_handle);
         message
     }
@@ -439,16 +443,15 @@ mod test {
             Ctap1Command::process_command(&mut env, &message, &mut ctap_state, CtapInstant::new(0))
                 .unwrap();
         assert_eq!(response[0], Ctap1Command::LEGACY_BYTE);
-        assert_eq!(response[66], CREDENTIAL_ID_SIZE as u8);
+        assert_eq!(response[66], ECDSA_CREDENTIAL_ID_SIZE as u8);
-        assert!(ctap_state
+        assert!(decrypt_credential_source(
-            .decrypt_credential_source(
+            &mut env,
-                &mut env,
+            response[67..67 + ECDSA_CREDENTIAL_ID_SIZE].to_vec(),
-                response[67..67 + CREDENTIAL_ID_SIZE].to_vec(),
+            &application
-                &application
+        )
-            )
+        .unwrap()
-            .unwrap()
+        .is_some());
-            .is_some());
+        const CERT_START: usize = 67 + ECDSA_CREDENTIAL_ID_SIZE;
-        const CERT_START: usize = 67 + CREDENTIAL_ID_SIZE;
         assert_eq!(
             &response[CERT_START..CERT_START + fake_cert.len()],
             &fake_cert[..]
@@ -498,14 +501,12 @@ mod test {
         let mut env = TestEnv::new();
         env.user_presence()
             .set(|_| panic!("Unexpected user presence check in CTAP1"));
-        let sk = crypto::ecdsa::SecKey::gensk(env.rng());
+        let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = ctap_state
+        let key_handle = encrypt_key_handle(&mut env, &sk, &application).unwrap();
-            .encrypt_key_handle(&mut env, sk, &application)
-            .unwrap();
         let message = create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle);
 
         let response =
@@ -518,14 +519,12 @@ mod test {
         let mut env = TestEnv::new();
         env.user_presence()
             .set(|_| panic!("Unexpected user presence check in CTAP1"));
-        let sk = crypto::ecdsa::SecKey::gensk(env.rng());
+        let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = ctap_state
+        let key_handle = encrypt_key_handle(&mut env, &sk, &application).unwrap();
-            .encrypt_key_handle(&mut env, sk, &application)
-            .unwrap();
         let application = [0x55; 32];
         let message = create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle);
 
@@ -539,14 +538,12 @@ mod test {
         let mut env = TestEnv::new();
         env.user_presence()
             .set(|_| panic!("Unexpected user presence check in CTAP1"));
-        let sk = crypto::ecdsa::SecKey::gensk(env.rng());
+        let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = ctap_state
+        let key_handle = encrypt_key_handle(&mut env, &sk, &application).unwrap();
-            .encrypt_key_handle(&mut env, sk, &application)
-            .unwrap();
         let mut message = create_authenticate_message(
             &application,
             Ctap1Flags::DontEnforceUpAndSign,
@@ -579,14 +576,12 @@ mod test {
         let mut env = TestEnv::new();
         env.user_presence()
             .set(|_| panic!("Unexpected user presence check in CTAP1"));
-        let sk = crypto::ecdsa::SecKey::gensk(env.rng());
+        let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = ctap_state
+        let key_handle = encrypt_key_handle(&mut env, &sk, &application).unwrap();
-            .encrypt_key_handle(&mut env, sk, &application)
-            .unwrap();
         let mut message =
             create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle);
         message[0] = 0xEE;
@@ -601,14 +596,12 @@ mod test {
         let mut env = TestEnv::new();
         env.user_presence()
             .set(|_| panic!("Unexpected user presence check in CTAP1"));
-        let sk = crypto::ecdsa::SecKey::gensk(env.rng());
+        let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = ctap_state
+        let key_handle = encrypt_key_handle(&mut env, &sk, &application).unwrap();
-            .encrypt_key_handle(&mut env, sk, &application)
-            .unwrap();
         let mut message =
             create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle);
         message[1] = 0xEE;
@@ -623,14 +616,12 @@ mod test {
         let mut env = TestEnv::new();
         env.user_presence()
             .set(|_| panic!("Unexpected user presence check in CTAP1"));
-        let sk = crypto::ecdsa::SecKey::gensk(env.rng());
+        let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = ctap_state
+        let key_handle = encrypt_key_handle(&mut env, &sk, &application).unwrap();
-            .encrypt_key_handle(&mut env, sk, &application)
-            .unwrap();
         let mut message =
             create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle);
         message[2] = 0xEE;
@@ -653,14 +644,12 @@ mod test {
         let mut env = TestEnv::new();
         env.user_presence()
             .set(|_| panic!("Unexpected user presence check in CTAP1"));
-        let sk = crypto::ecdsa::SecKey::gensk(env.rng());
+        let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = ctap_state
+        let key_handle = encrypt_key_handle(&mut env, &sk, &application).unwrap();
-            .encrypt_key_handle(&mut env, sk, &application)
-            .unwrap();
         let message =
             create_authenticate_message(&application, Ctap1Flags::EnforceUpAndSign, &key_handle);
 
@@ -683,14 +672,12 @@ mod test {
         let mut env = TestEnv::new();
         env.user_presence()
             .set(|_| panic!("Unexpected user presence check in CTAP1"));
-        let sk = crypto::ecdsa::SecKey::gensk(env.rng());
+        let sk = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256);
         let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = ctap_state
+        let key_handle = encrypt_key_handle(&mut env, &sk, &application).unwrap();
-            .encrypt_key_handle(&mut env, sk, &application)
-            .unwrap();
         let message = create_authenticate_message(
             &application,
             Ctap1Flags::DontEnforceUpAndSign,
@@ -716,7 +703,7 @@ mod test {
     #[test]
     fn test_process_authenticate_bad_key_handle() {
         let application = [0x0A; 32];
-        let key_handle = vec![0x00; CREDENTIAL_ID_SIZE];
+        let key_handle = vec![0x00; ECDSA_CREDENTIAL_ID_SIZE];
         let message =
             create_authenticate_message(&application, Ctap1Flags::EnforceUpAndSign, &key_handle);
 
@@ -735,7 +722,7 @@ mod test {
     #[test]
     fn test_process_authenticate_without_up() {
         let application = [0x0A; 32];
-        let key_handle = vec![0x00; CREDENTIAL_ID_SIZE];
+        let key_handle = vec![0x00; ECDSA_CREDENTIAL_ID_SIZE];
         let message =
             create_authenticate_message(&application, Ctap1Flags::EnforceUpAndSign, &key_handle);
 

src/ctap/data_formats.rs

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use super::crypto_wrapper::PrivateKey;
 use super::status_code::Ctap2StatusCode;
 use alloc::string::String;
 use alloc::vec::Vec;
@@ -497,7 +498,8 @@ impl From<PackedAttestationStatement> for cbor::Value {
     }
 }
 
-#[derive(Clone, Debug, PartialEq, Eq)]
+/// Signature algorithm identifier, as specified for COSE.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
 #[cfg_attr(feature = "fuzz", derive(Arbitrary))]
 pub enum SignatureAlgorithm {
     ES256 = ES256_ALGORITHM as isize,
@@ -512,14 +514,20 @@ impl From<SignatureAlgorithm> for cbor::Value {
     }
 }
 
+impl From<i64> for SignatureAlgorithm {
+    fn from(int: i64) -> Self {
+        match int {
+            ES256_ALGORITHM => SignatureAlgorithm::ES256,
+            _ => SignatureAlgorithm::Unknown,
+        }
+    }
+}
+
 impl TryFrom<cbor::Value> for SignatureAlgorithm {
     type Error = Ctap2StatusCode;
 
     fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
-        match extract_integer(cbor_value)? {
+        extract_integer(cbor_value).map(SignatureAlgorithm::from)
-            ES256_ALGORITHM => Ok(SignatureAlgorithm::ES256),
-            _ => Ok(SignatureAlgorithm::Unknown),
-        }
     }
 }
 
@@ -565,10 +573,9 @@ impl TryFrom<cbor::Value> for CredentialProtectionPolicy {
 // by FIDO. In particular we may choose how we serialize and deserialize it.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct PublicKeyCredentialSource {
-    // TODO function to convert to / from Vec<u8>
     pub key_type: PublicKeyCredentialType,
     pub credential_id: Vec<u8>,
-    pub private_key: ecdsa::SecKey, // TODO(kaczmarczyck) open for other algorithms
+    pub private_key: PrivateKey,
     pub rp_id: String,
     pub user_handle: Vec<u8>, // not optional, but nullable
     pub user_display_name: Option<String>,
@@ -584,7 +591,8 @@ pub struct PublicKeyCredentialSource {
 // is associated with a unique tag, implemented with a CBOR unsigned key.
 enum PublicKeyCredentialSourceField {
     CredentialId = 0,
-    PrivateKey = 1,
+    // Deprecated, we still read this field for backwards compatibility.
+    EcdsaPrivateKey = 1,
     RpId = 2,
     UserHandle = 3,
     UserDisplayName = 4,
@@ -594,6 +602,7 @@ enum PublicKeyCredentialSourceField {
     UserIcon = 9,
     CredBlob = 10,
     LargeBlobKey = 11,
+    PrivateKey = 12,
     // When a field is removed, its tag should be reserved and not used for new fields. We document
     // those reserved tags below.
     // Reserved tags:
@@ -608,11 +617,8 @@ impl From<PublicKeyCredentialSourceField> for cbor::Value {
 
 impl From<PublicKeyCredentialSource> for cbor::Value {
     fn from(credential: PublicKeyCredentialSource) -> cbor::Value {
-        let mut private_key = [0u8; 32];
-        credential.private_key.to_bytes(&mut private_key);
         cbor_map_options! {
             PublicKeyCredentialSourceField::CredentialId => Some(credential.credential_id),
-            PublicKeyCredentialSourceField::PrivateKey => Some(private_key.to_vec()),
             PublicKeyCredentialSourceField::RpId => Some(credential.rp_id),
             PublicKeyCredentialSourceField::UserHandle => Some(credential.user_handle),
             PublicKeyCredentialSourceField::UserDisplayName => credential.user_display_name,
@@ -622,6 +628,7 @@ impl From<PublicKeyCredentialSource> for cbor::Value {
             PublicKeyCredentialSourceField::UserIcon => credential.user_icon,
             PublicKeyCredentialSourceField::CredBlob => credential.cred_blob,
             PublicKeyCredentialSourceField::LargeBlobKey => credential.large_blob_key,
+            PublicKeyCredentialSourceField::PrivateKey => credential.private_key,
         }
     }
 }
@@ -633,7 +640,7 @@ impl TryFrom<cbor::Value> for PublicKeyCredentialSource {
         destructure_cbor_map! {
             let {
                 PublicKeyCredentialSourceField::CredentialId => credential_id,
-                PublicKeyCredentialSourceField::PrivateKey => private_key,
+                PublicKeyCredentialSourceField::EcdsaPrivateKey => ecdsa_private_key,
                 PublicKeyCredentialSourceField::RpId => rp_id,
                 PublicKeyCredentialSourceField::UserHandle => user_handle,
                 PublicKeyCredentialSourceField::UserDisplayName => user_display_name,
@@ -643,16 +650,11 @@ impl TryFrom<cbor::Value> for PublicKeyCredentialSource {
                 PublicKeyCredentialSourceField::UserIcon => user_icon,
                 PublicKeyCredentialSourceField::CredBlob => cred_blob,
                 PublicKeyCredentialSourceField::LargeBlobKey => large_blob_key,
+                PublicKeyCredentialSourceField::PrivateKey => private_key,
             } = extract_map(cbor_value)?;
         }
 
         let credential_id = extract_byte_string(ok_or_missing(credential_id)?)?;
-        let private_key = extract_byte_string(ok_or_missing(private_key)?)?;
-        if private_key.len() != 32 {
-            return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR);
-        }
-        let private_key = ecdsa::SecKey::from_bytes(array_ref!(private_key, 0, 32))
-            .ok_or(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR)?;
         let rp_id = extract_text_string(ok_or_missing(rp_id)?)?;
         let user_handle = extract_byte_string(ok_or_missing(user_handle)?)?;
         let user_display_name = user_display_name.map(extract_text_string).transpose()?;
@@ -664,6 +666,18 @@ impl TryFrom<cbor::Value> 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,
+        };
+
         // We don't return whether there were unknown fields in the CBOR value. This means that
         // deserialization is not injective. In particular deserialization is only an inverse of
         // serialization at a given version of OpenSK. This is not a problem because:
@@ -1544,6 +1558,17 @@ mod test {
         assert_eq!(unknown_type, Ok(expected_unknown_type));
     }
 
+    #[test]
+    fn test_from_into_signature_algorithm_int() {
+        let alg_int = SignatureAlgorithm::ES256 as i64;
+        let signature_algorithm = SignatureAlgorithm::from(alg_int);
+        assert_eq!(signature_algorithm, SignatureAlgorithm::ES256);
+
+        let unknown_alg_int = -1;
+        let unknown_algorithm = SignatureAlgorithm::from(unknown_alg_int);
+        assert_eq!(unknown_algorithm, SignatureAlgorithm::Unknown);
+    }
+
     #[test]
     fn test_from_into_signature_algorithm() {
         let cbor_signature_algorithm: cbor::Value = cbor_int!(ES256_ALGORITHM);
@@ -2108,10 +2133,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 credential = PublicKeyCredentialSource {
             key_type: PublicKeyCredentialType::PublicKey,
             credential_id: env.rng().gen_uniform_u8x32().to_vec(),
-            private_key: crypto::ecdsa::SecKey::gensk(env.rng()),
+            private_key: PrivateKey::from(private_key),
             rp_id: "example.com".to_string(),
             user_handle: b"foo".to_vec(),
             user_display_name: None,
@@ -2189,6 +2215,83 @@ 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 credential = PublicKeyCredentialSource {
+            key_type: PublicKeyCredentialType::PublicKey,
+            credential_id: env.rng().gen_uniform_u8x32().to_vec(),
+            private_key: PrivateKey::from(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),
+            Ok(credential)
+        );
+    }
+
+    #[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 credential = PublicKeyCredentialSource {
+            key_type: PublicKeyCredentialType::PublicKey,
+            credential_id: env.rng().gen_uniform_u8x32().to_vec(),
+            private_key: PrivateKey::from(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 => PrivateKey::from(private_key),
+        };
+        assert_eq!(
+            PublicKeyCredentialSource::try_from(source_cbor),
+            Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)
+        );
+    }
+
     #[test]
     fn test_credential_source_invalid_cbor() {
         assert!(PublicKeyCredentialSource::try_from(cbor_false!()).is_err());

src/ctap/mod.rs

@@ -41,7 +41,9 @@ use self::command::{
 };
 use self::config_command::process_config;
 use self::credential_management::process_credential_management;
-use self::crypto_wrapper::{aes256_cbc_decrypt, aes256_cbc_encrypt};
+use self::crypto_wrapper::{
+    decrypt_credential_source, encrypt_key_handle, PrivateKey, MAX_CREDENTIAL_ID_SIZE,
+};
 use self::data_formats::{
     AuthenticatorTransport, CoseKey, CoseSignature, CredentialProtectionPolicy,
     EnterpriseAttestationMode, GetAssertionExtensions, PackedAttestationStatement,
@@ -72,7 +74,7 @@ use alloc::vec::Vec;
 use arrayref::array_ref;
 use byteorder::{BigEndian, ByteOrder};
 use core::convert::TryFrom;
-use crypto::hmac::{hmac_256, verify_hmac_256};
+use crypto::hmac::hmac_256;
 use crypto::sha256::Sha256;
 use crypto::{ecdsa, Hash256};
 use embedded_time::duration::Milliseconds;
@@ -81,12 +83,6 @@ use sk_cbor as cbor;
 use sk_cbor::cbor_map_options;
 
 pub const INITIAL_SIGNATURE_COUNTER: u32 = 1;
-// Our credential ID consists of
-// - 16 byte initialization vector for AES-256,
-// - 32 byte ECDSA private key for the credential,
-// - 32 byte relying party ID hashed with SHA256,
-// - 32 byte HMAC-SHA256 over everything else.
-pub const CREDENTIAL_ID_SIZE: usize = 112;
 // Set this bit when checking user presence.
 const UP_FLAG: u8 = 0x01;
 // Set this bit when checking user verification.
@@ -431,72 +427,6 @@ impl CtapState {
         Ok(!storage::has_always_uv(env)?)
     }
 
-    // Encrypts the private key and relying party ID hash into a credential ID. Other
-    // information, such as a user name, are not stored, because encrypted credential IDs
-    // are used for credentials stored server-side. Also, we want the key handle to be
-    // compatible with U2F.
-    pub fn encrypt_key_handle(
-        &mut self,
-        env: &mut impl Env,
-        private_key: crypto::ecdsa::SecKey,
-        application: &[u8; 32],
-    ) -> Result<Vec<u8>, Ctap2StatusCode> {
-        let master_keys = storage::master_keys(env)?;
-        let aes_enc_key = crypto::aes256::EncryptionKey::new(&master_keys.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::<Sha256>(&master_keys.hmac, &encrypted_id[..]);
-        encrypted_id.extend(&id_hmac);
-        Ok(encrypted_id)
-    }
-
-    // Decrypts a credential ID and writes the private key into a PublicKeyCredentialSource.
-    // None is returned if the HMAC test fails or the relying party does not match the
-    // decrypted relying party ID hash.
-    pub fn decrypt_credential_source(
-        &self,
-        env: &mut impl Env,
-        credential_id: Vec<u8>,
-        rp_id_hash: &[u8],
-    ) -> Result<Option<PublicKeyCredentialSource>, Ctap2StatusCode> {
-        if credential_id.len() != CREDENTIAL_ID_SIZE {
-            return Ok(None);
-        }
-        let master_keys = storage::master_keys(env)?;
-        let payload_size = credential_id.len() - 32;
-        if !verify_hmac_256::<Sha256>(
-            &master_keys.hmac,
-            &credential_id[..payload_size],
-            array_ref![credential_id, payload_size, 32],
-        ) {
-            return Ok(None);
-        }
-        let aes_enc_key = crypto::aes256::EncryptionKey::new(&master_keys.encryption);
-
-        let decrypted_id = aes256_cbc_decrypt(&aes_enc_key, &credential_id[..payload_size], true)?;
-        if rp_id_hash != &decrypted_id[32..64] {
-            return Ok(None);
-        }
-        let sk_option = crypto::ecdsa::SecKey::from_bytes(array_ref!(decrypted_id, 0, 32));
-        Ok(sk_option.map(|sk| PublicKeyCredentialSource {
-            key_type: PublicKeyCredentialType::PublicKey,
-            credential_id,
-            private_key: sk,
-            rp_id: String::from(""),
-            user_handle: 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,
-        }))
-    }
-
     pub fn process_command(
         &mut self,
         env: &mut impl Env,
@@ -673,9 +603,11 @@ impl CtapState {
 
         self.pin_uv_auth_precheck(env, &pin_uv_auth_param, pin_uv_auth_protocol, channel)?;
 
+        // When more algorithms are supported, iterate and pick the first match.
         if !pub_key_cred_params.contains(&ES256_CRED_PARAM) {
             return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM);
         }
+        let algorithm = SignatureAlgorithm::ES256;
 
         let rp_id = rp.rp_id;
         let ep_att = if let Some(enterprise_attestation) = enterprise_attestation {
@@ -744,9 +676,7 @@ impl CtapState {
         if let Some(exclude_list) = exclude_list {
             for cred_desc in exclude_list {
                 if storage::find_credential(env, &rp_id, &cred_desc.key_id, !has_uv)?.is_some()
-                    || self
+                    || decrypt_credential_source(env, cred_desc.key_id, &rp_id_hash)?.is_some()
-                        .decrypt_credential_source(env, cred_desc.key_id, &rp_id_hash)?
-                        .is_some()
                 {
                     // Perform this check, so bad actors can't brute force exclude_list
                     // without user interaction.
@@ -790,15 +720,15 @@ impl CtapState {
             _ => None,
         };
 
-        let sk = crypto::ecdsa::SecKey::gensk(env.rng());
+        // We decide on the algorithm early, but delay key creation since it takes time.
-        let pk = sk.genpk();
+        // We rather do that later so all intermediate checks may return faster.
-
+        let private_key = PrivateKey::new(env.rng(), algorithm);
         let credential_id = if options.rk {
             let random_id = env.rng().gen_uniform_u8x32().to_vec();
             let credential_source = PublicKeyCredentialSource {
                 key_type: PublicKeyCredentialType::PublicKey,
                 credential_id: random_id.clone(),
-                private_key: sk.clone(),
+                private_key: private_key.clone(),
                 rp_id,
                 user_handle: user.user_id,
                 // This input is user provided, so we crop it to 64 byte for storage.
@@ -820,18 +750,19 @@ impl CtapState {
             storage::store_credential(env, credential_source)?;
             random_id
         } else {
-            self.encrypt_key_handle(env, sk.clone(), &rp_id_hash)?
+            encrypt_key_handle(env, &private_key, &rp_id_hash)?
         };
 
         let mut auth_data = self.generate_auth_data(env, &rp_id_hash, flags)?;
         auth_data.extend(&storage::aaguid(env)?);
-        // The length is fixed to 0x20 or 0x70 and fits one byte.
+        // The length is fixed to 0x20 or 0x80 and fits one byte.
         if credential_id.len() > 0xFF {
             return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR);
         }
         auth_data.extend(vec![0x00, credential_id.len() as u8]);
         auth_data.extend(&credential_id);
-        cbor_write(cbor::Value::from(CoseKey::from(pk)), &mut auth_data)?;
+        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 {
                 Some(true)
@@ -864,15 +795,17 @@ impl CtapState {
             let attestation_certificate = storage::attestation_certificate(env)?
                 .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?;
             (
-                attestation_key.sign_rfc6979::<Sha256>(&signature_data),
+                attestation_key
+                    .sign_rfc6979::<Sha256>(&signature_data)
+                    .to_asn1_der(),
                 Some(vec![attestation_certificate]),
             )
         } else {
-            (sk.sign_rfc6979::<Sha256>(&signature_data), None)
+            (private_key.sign_and_encode(&signature_data), None)
         };
         let attestation_statement = PackedAttestationStatement {
             alg: SignatureAlgorithm::ES256 as i64,
-            sig: signature.to_asn1_der(),
+            sig: signature,
             x5c,
             ecdaa_key_id: None,
         };
@@ -893,13 +826,12 @@ impl CtapState {
     fn generate_cred_random(
         &mut self,
         env: &mut impl Env,
-        private_key: &crypto::ecdsa::SecKey,
+        private_key: &PrivateKey,
         has_uv: bool,
     ) -> Result<[u8; 32], Ctap2StatusCode> {
-        let mut private_key_bytes = [0u8; 32];
+        let entropy = private_key.to_bytes();
-        private_key.to_bytes(&mut private_key_bytes);
         let key = storage::cred_random_secret(env, has_uv)?;
-        Ok(hmac_256::<Sha256>(&key, &private_key_bytes))
+        Ok(hmac_256::<Sha256>(&key, &entropy))
     }
 
     // Processes the input of a get_assertion operation for a given credential
@@ -951,9 +883,7 @@ impl CtapState {
 
         let mut signature_data = auth_data.clone();
         signature_data.extend(client_data_hash);
-        let signature = credential
+        let signature = credential.private_key.sign_and_encode(&signature_data);
-            .private_key
-            .sign_rfc6979::<Sha256>(&signature_data);
 
         let cred_desc = PublicKeyCredentialDescriptor {
             key_type: PublicKeyCredentialType::PublicKey,
@@ -979,7 +909,7 @@ impl CtapState {
         let response_data = AuthenticatorGetAssertionResponse {
             credential: Some(cred_desc),
             auth_data,
-            signature: signature.to_asn1_der(),
+            signature,
             user,
             number_of_credentials: number_of_credentials.map(|n| n as u64),
             large_blob_key,
@@ -1007,8 +937,7 @@ impl CtapState {
             if credential.is_some() {
                 return Ok(credential);
             }
-            let credential =
+            let credential = decrypt_credential_source(env, allowed_credential.key_id, rp_id_hash)?;
-                self.decrypt_credential_source(env, allowed_credential.key_id, rp_id_hash)?;
             if credential.is_some() {
                 return Ok(credential);
             }
@@ -1215,7 +1144,7 @@ impl CtapState {
                     .customization()
                     .max_credential_count_in_list()
                     .map(|c| c as u64),
-                max_credential_id_length: Some(CREDENTIAL_ID_SIZE as u64),
+                max_credential_id_length: Some(MAX_CREDENTIAL_ID_SIZE as u64),
                 transports: Some(vec![AuthenticatorTransport::Usb]),
                 algorithms: Some(vec![ES256_CRED_PARAM]),
                 max_serialized_large_blob_array: Some(
@@ -1429,6 +1358,7 @@ mod test {
         AuthenticatorAttestationMaterial, AuthenticatorClientPinParameters,
         AuthenticatorCredentialManagementParameters,
     };
+    use super::crypto_wrapper::ECDSA_CREDENTIAL_ID_SIZE;
     use super::data_formats::{
         ClientPinSubCommand, CoseKey, CredentialManagementSubCommand, GetAssertionHmacSecretInput,
         GetAssertionOptions, MakeCredentialExtensions, MakeCredentialOptions, PinUvAuthProtocol,
@@ -1529,7 +1459,7 @@ mod test {
             0x05 => env.customization().max_msg_size() as u64,
             0x06 => cbor_array![2, 1],
             0x07 => env.customization().max_credential_count_in_list().map(|c| c as u64),
-            0x08 => CREDENTIAL_ID_SIZE as u64,
+            0x08 => MAX_CREDENTIAL_ID_SIZE as u64,
             0x09 => cbor_array!["usb"],
             0x0A => cbor_array![ES256_CRED_PARAM],
             0x0B => env.customization().max_large_blob_array_size() as u64,
@@ -1635,7 +1565,7 @@ mod test {
             make_credential_response,
             0x41,
             &storage::aaguid(&mut env).unwrap(),
-            CREDENTIAL_ID_SIZE as u8,
+            ECDSA_CREDENTIAL_ID_SIZE as u8,
             &[],
         );
     }
@@ -1668,7 +1598,7 @@ mod test {
         let excluded_credential_source = PublicKeyCredentialSource {
             key_type: PublicKeyCredentialType::PublicKey,
             credential_id: excluded_credential_id,
-            private_key: excluded_private_key,
+            private_key: PrivateKey::from(excluded_private_key),
             rp_id: String::from("example.com"),
             user_handle: vec![],
             user_display_name: None,
@@ -1762,7 +1692,7 @@ mod test {
             make_credential_response,
             0xC1,
             &storage::aaguid(&mut env).unwrap(),
-            CREDENTIAL_ID_SIZE as u8,
+            ECDSA_CREDENTIAL_ID_SIZE as u8,
             &expected_extension_cbor,
         );
     }
@@ -2005,7 +1935,7 @@ mod test {
             make_credential_response,
             0x41,
             &storage::aaguid(&mut env).unwrap(),
-            0x70,
+            ECDSA_CREDENTIAL_ID_SIZE as u8,
             &[],
         );
     }
@@ -2374,8 +2304,8 @@ mod test {
                 let auth_data = make_credential_response.auth_data;
                 let offset = 37 + storage::aaguid(&mut env).unwrap().len();
                 assert_eq!(auth_data[offset], 0x00);
-                assert_eq!(auth_data[offset + 1] as usize, CREDENTIAL_ID_SIZE);
+                assert_eq!(auth_data[offset + 1] as usize, ECDSA_CREDENTIAL_ID_SIZE);
-                auth_data[offset + 2..offset + 2 + CREDENTIAL_ID_SIZE].to_vec()
+                auth_data[offset + 2..offset + 2 + ECDSA_CREDENTIAL_ID_SIZE].to_vec()
             }
             _ => panic!("Invalid response type"),
         };
@@ -2490,7 +2420,7 @@ mod test {
         let credential = PublicKeyCredentialSource {
             key_type: PublicKeyCredentialType::PublicKey,
             credential_id: credential_id.clone(),
-            private_key: private_key.clone(),
+            private_key: PrivateKey::from(private_key.clone()),
             rp_id: String::from("example.com"),
             user_handle: vec![0x1D],
             user_display_name: None,
@@ -2552,7 +2482,7 @@ mod test {
         let credential = PublicKeyCredentialSource {
             key_type: PublicKeyCredentialType::PublicKey,
             credential_id,
-            private_key,
+            private_key: PrivateKey::from(private_key),
             rp_id: String::from("example.com"),
             user_handle: vec![0x1D],
             user_display_name: None,
@@ -2599,7 +2529,7 @@ mod test {
         let credential = PublicKeyCredentialSource {
             key_type: PublicKeyCredentialType::PublicKey,
             credential_id,
-            private_key,
+            private_key: PrivateKey::from(private_key),
             rp_id: String::from("example.com"),
             user_handle: vec![0x1D],
             user_display_name: None,
@@ -2657,7 +2587,7 @@ mod test {
         let credential = PublicKeyCredentialSource {
             key_type: PublicKeyCredentialType::PublicKey,
             credential_id,
-            private_key,
+            private_key: PrivateKey::from(private_key),
             rp_id: String::from("example.com"),
             user_handle: vec![0x1D],
             user_display_name: None,
@@ -2943,7 +2873,7 @@ mod test {
         let credential_source = PublicKeyCredentialSource {
             key_type: PublicKeyCredentialType::PublicKey,
             credential_id,
-            private_key,
+            private_key: PrivateKey::from(private_key),
             rp_id: String::from("example.com"),
             user_handle: vec![],
             user_display_name: None,
@@ -3019,47 +2949,6 @@ mod test {
         assert_eq!(reponse, expected_response);
     }
 
-    #[test]
-    fn test_encrypt_decrypt_credential() {
-        let mut env = TestEnv::new();
-        let private_key = crypto::ecdsa::SecKey::gensk(env.rng());
-        let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
-
-        // Usually, the relying party ID or its hash is provided by the client.
-        // We are not testing the correctness of our SHA256 here, only if it is checked.
-        let rp_id_hash = [0x55; 32];
-        let encrypted_id = ctap_state
-            .encrypt_key_handle(&mut env, private_key.clone(), &rp_id_hash)
-            .unwrap();
-        let decrypted_source = ctap_state
-            .decrypt_credential_source(&mut env, encrypted_id, &rp_id_hash)
-            .unwrap()
-            .unwrap();
-
-        assert_eq!(private_key, decrypted_source.private_key);
-    }
-
-    #[test]
-    fn test_encrypt_decrypt_bad_hmac() {
-        let mut env = TestEnv::new();
-        let private_key = crypto::ecdsa::SecKey::gensk(env.rng());
-        let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
-
-        // Same as above.
-        let rp_id_hash = [0x55; 32];
-        let encrypted_id = ctap_state
-            .encrypt_key_handle(&mut env, private_key, &rp_id_hash)
-            .unwrap();
-        for i in 0..encrypted_id.len() {
-            let mut modified_id = encrypted_id.clone();
-            modified_id[i] ^= 0x01;
-            assert!(ctap_state
-                .decrypt_credential_source(&mut env, modified_id, &rp_id_hash)
-                .unwrap()
-                .is_none());
-        }
-    }
-
     #[test]
     fn test_signature_counter() {
         let mut env = TestEnv::new();
@@ -3483,10 +3372,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 credential_source = PublicKeyCredentialSource {
             key_type: PublicKeyCredentialType::PublicKey,
             credential_id: env.rng().gen_uniform_u8x32().to_vec(),
-            private_key: crypto::ecdsa::SecKey::gensk(env.rng()),
+            private_key: PrivateKey::from(private_key),
             rp_id: String::from("example.com"),
             user_handle: vec![0x01],
             user_display_name: Some("display_name".to_string()),

src/ctap/storage.rs

@@ -728,6 +728,7 @@ fn serialize_min_pin_length_rp_ids(rp_ids: Vec<String>) -> Result<Vec<u8>, Ctap2
 #[cfg(test)]
 mod test {
     use super::*;
+    use crate::ctap::crypto_wrapper::PrivateKey;
     use crate::ctap::data_formats::{PublicKeyCredentialSource, PublicKeyCredentialType};
     use crate::env::test::TestEnv;
     use rng256::Rng256;
@@ -741,7 +742,7 @@ mod test {
         PublicKeyCredentialSource {
             key_type: PublicKeyCredentialType::PublicKey,
             credential_id: rng.gen_uniform_u8x32().to_vec(),
-            private_key,
+            private_key: PrivateKey::from(private_key),
             rp_id: String::from(rp_id),
             user_handle,
             user_display_name: None,
@@ -963,7 +964,7 @@ mod test {
         let credential = PublicKeyCredentialSource {
             key_type: PublicKeyCredentialType::PublicKey,
             credential_id: env.rng().gen_uniform_u8x32().to_vec(),
-            private_key,
+            private_key: PrivateKey::from(private_key),
             rp_id: String::from("example.com"),
             user_handle: vec![0x00],
             user_display_name: None,
@@ -1284,7 +1285,7 @@ mod test {
         let credential = PublicKeyCredentialSource {
             key_type: PublicKeyCredentialType::PublicKey,
             credential_id: env.rng().gen_uniform_u8x32().to_vec(),
-            private_key,
+            private_key: PrivateKey::from(private_key),
             rp_id: String::from("example.com"),
             user_handle: vec![0x00],
             user_display_name: Some(String::from("Display Name")),