Add CBOR credential ID type (#512)

* Add CBOR credential ID type

Update the format of the credential ID we generated to extend the
encrypted portion from only private_key + rp_id_hash to a flexible CBOR
map. This way we can persist more data into the key handle in the future
without need of a new version.

We add credProtectPolicy to the persisted data in this commit too, so we
can correctly check the credProtectPolicy for non-discoverable
credentials in follow-up commits.

* Fixed some style problems.

* Fix cargo clippy warning

* Check credProtectPolicy for non-discoverable credentials.

* Remove support of old v1, v2 key handles

- And changed some style problems

* Style changes

* Add missing `alloc` use

src/ctap/credential_id.rs

@@ -0,0 +1,475 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use super::crypto_wrapper::{aes256_cbc_decrypt, aes256_cbc_encrypt, PrivateKey};
+use super::data_formats::{
+    CredentialProtectionPolicy, PublicKeyCredentialSource, PublicKeyCredentialType,
+};
+use super::status_code::Ctap2StatusCode;
+use super::{cbor_read, cbor_write};
+use crate::api::key_store::KeyStore;
+use crate::ctap::data_formats::{extract_byte_string, extract_map};
+use crate::env::Env;
+use alloc::string::String;
+use alloc::vec::Vec;
+use core::convert::{TryFrom, TryInto};
+use crypto::hmac::{hmac_256, verify_hmac_256};
+use crypto::sha256::Sha256;
+use sk_cbor::{cbor_map_options, destructure_cbor_map};
+
+pub 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;
+pub const MIN_CREDENTIAL_ID_SIZE: usize = LEGACY_CREDENTIAL_ID_SIZE;
+pub const MAX_CREDENTIAL_ID_SIZE: usize = CBOR_CREDENTIAL_ID_SIZE;
+
+pub const CBOR_CREDENTIAL_ID_VERSION: u8 = 0x01;
+
+pub const MAX_PADDING_LENGTH: u8 = 0xBF;
+
+// Data fields that are contained in the credential ID of non-discoverable credentials.
+struct CredentialSource {
+    private_key: PrivateKey,
+    rp_id_hash: [u8; 32],
+    cred_protect_policy: Option<CredentialProtectionPolicy>,
+}
+
+// The data fields contained in the credential ID are serizlied using CBOR maps.
+// Each field is associated with a unique tag, implemented with a CBOR unsigned key.
+enum CredentialSourceField {
+    PrivateKey = 0,
+    RpIdHash = 1,
+    CredProtectPolicy = 2,
+}
+
+impl From<CredentialSourceField> for sk_cbor::Value {
+    fn from(field: CredentialSourceField) -> sk_cbor::Value {
+        (field as u64).into()
+    }
+}
+
+fn decrypt_legacy_credential_id(
+    env: &mut impl Env,
+    bytes: &[u8],
+) -> Result<Option<CredentialSource>, Ctap2StatusCode> {
+    let aes_enc_key = crypto::aes256::EncryptionKey::new(&env.key_store().key_handle_encryption()?);
+    let plaintext = aes256_cbc_decrypt(&aes_enc_key, bytes, true)?;
+    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,
+    }))
+}
+
+fn decrypt_cbor_credential_id(
+    env: &mut impl Env,
+    bytes: &[u8],
+) -> Result<Option<CredentialSource>, Ctap2StatusCode> {
+    let aes_enc_key = crypto::aes256::EncryptionKey::new(&env.key_store().key_handle_encryption()?);
+    let mut plaintext = aes256_cbc_decrypt(&aes_enc_key, bytes, true)?;
+    remove_padding(&mut plaintext)?;
+
+    let cbor_credential_source = cbor_read(plaintext.as_slice())?;
+    destructure_cbor_map! {
+      let {
+          CredentialSourceField::PrivateKey => private_key,
+          CredentialSourceField::RpIdHash=> rp_id_hash,
+          CredentialSourceField::CredProtectPolicy => cred_protect_policy,
+      } = extract_map(cbor_credential_source)?;
+    }
+    Ok(match (private_key, rp_id_hash, cred_protect_policy) {
+        (Some(private_key), Some(rp_id_hash), cred_protect_policy) => {
+            let private_key = PrivateKey::try_from(private_key)?;
+            let rp_id_hash = extract_byte_string(rp_id_hash)?;
+            if rp_id_hash.len() != 32 {
+                return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR);
+            }
+            let cred_protect_policy = if let Some(policy) = cred_protect_policy {
+                Some(CredentialProtectionPolicy::try_from(policy)?)
+            } else {
+                None
+            };
+            Some(CredentialSource {
+                private_key,
+                rp_id_hash: rp_id_hash.try_into().unwrap(),
+                cred_protect_policy,
+            })
+        }
+        _ => None,
+    })
+}
+
+/// Pad data to MAX_PADDING_LENGTH+1 (192) bytes using PKCS padding scheme.
+/// Let N = 192 - data.len(), the PKCS padding scheme would pad N bytes of N after the data.
+fn add_padding(data: &mut Vec<u8>) -> Result<(), Ctap2StatusCode> {
+    // The data should be between 1 to MAX_PADDING_LENGTH bytes for the padding scheme to be valid.
+    if data.is_empty() || data.len() > MAX_PADDING_LENGTH as usize {
+        return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR);
+    }
+    let pad_length = MAX_PADDING_LENGTH - (data.len() as u8 - 1);
+    data.extend(core::iter::repeat(pad_length).take(pad_length as usize));
+    Ok(())
+}
+
+fn remove_padding(data: &mut Vec<u8>) -> Result<(), Ctap2StatusCode> {
+    if data.len() != MAX_PADDING_LENGTH as usize + 1 {
+        // This is an internal error instead of corrupted credential ID which we should just ignore because
+        // we've already checked that the HMAC matched.
+        return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR);
+    }
+    let pad_length = *data.last().unwrap();
+    if pad_length == 0 || pad_length > MAX_PADDING_LENGTH {
+        return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR);
+    }
+    if !data
+        .drain((data.len() - pad_length as usize)..)
+        .all(|x| x == pad_length)
+    {
+        return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR);
+    }
+    Ok(())
+}
+
+/// Encrypts the given private key, relying party ID hash, and cred protect policy 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).
+pub fn encrypt_to_credential_id(
+    env: &mut impl Env,
+    private_key: &PrivateKey,
+    rp_id_hash: &[u8; 32],
+    cred_protect_policy: Option<CredentialProtectionPolicy>,
+) -> Result<Vec<u8>, Ctap2StatusCode> {
+    let mut payload = Vec::new();
+    let cbor = cbor_map_options! {
+      CredentialSourceField::PrivateKey => private_key,
+      CredentialSourceField::RpIdHash=> rp_id_hash,
+      CredentialSourceField::CredProtectPolicy => cred_protect_policy,
+    };
+    cbor_write(cbor, &mut payload)?;
+    add_padding(&mut payload)?;
+
+    let aes_enc_key = crypto::aes256::EncryptionKey::new(&env.key_store().key_handle_encryption()?);
+    let encrypted_payload = aes256_cbc_encrypt(env.rng(), &aes_enc_key, &payload, true)?;
+    let mut credential_id = encrypted_payload;
+    credential_id.insert(0, CBOR_CREDENTIAL_ID_VERSION);
+
+    let id_hmac = hmac_256::<Sha256>(
+        &env.key_store().key_handle_authentication()?,
+        &credential_id[..],
+    );
+    credential_id.extend(&id_hmac);
+    Ok(credential_id)
+}
+
+/// Decrypts the given credential ID into a PublicKeyCredentialSource, populating only the recorded fields.
+///
+/// Returns None if
+/// - 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,
+/// - 192 bytes: encrypted CBOR-encoded credential source fields,
+/// -  32 bytes: HMAC-SHA256 over everything else.
+pub fn decrypt_credential_id(
+    env: &mut impl Env,
+    credential_id: Vec<u8>,
+    rp_id_hash: &[u8],
+    check_cred_protect: bool,
+) -> Result<Option<PublicKeyCredentialSource>, Ctap2StatusCode> {
+    if credential_id.len() < MIN_CREDENTIAL_ID_SIZE {
+        return Ok(None);
+    }
+    let hmac_message_size = credential_id.len() - 32;
+    if !verify_hmac_256::<Sha256>(
+        &env.key_store().key_handle_authentication()?,
+        &credential_id[..hmac_message_size],
+        array_ref![credential_id, hmac_message_size, 32],
+    ) {
+        return Ok(None);
+    }
+
+    let credential_source = if credential_id.len() == LEGACY_CREDENTIAL_ID_SIZE {
+        decrypt_legacy_credential_id(env, &credential_id[..hmac_message_size])?
+    } else {
+        match credential_id[0] {
+            CBOR_CREDENTIAL_ID_VERSION => {
+                if credential_id.len() != CBOR_CREDENTIAL_ID_SIZE {
+                    return Ok(None);
+                }
+                decrypt_cbor_credential_id(env, &credential_id[1..hmac_message_size])?
+            }
+            _ => return Ok(None),
+        }
+    };
+
+    let credential_source = if let Some(credential_source) = credential_source {
+        credential_source
+    } else {
+        return Ok(None);
+    };
+
+    let is_protected = credential_source.cred_protect_policy
+        == Some(CredentialProtectionPolicy::UserVerificationRequired);
+    if rp_id_hash != credential_source.rp_id_hash || (check_cred_protect && is_protected) {
+        return Ok(None);
+    }
+
+    Ok(Some(PublicKeyCredentialSource {
+        key_type: PublicKeyCredentialType::PublicKey,
+        credential_id,
+        private_key: credential_source.private_key,
+        rp_id: String::new(),
+        user_handle: Vec::new(),
+        user_display_name: None,
+        cred_protect_policy: credential_source.cred_protect_policy,
+        creation_order: 0,
+        user_name: None,
+        user_icon: None,
+        cred_blob: None,
+        large_blob_key: None,
+    }))
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::ctap::credential_id::CBOR_CREDENTIAL_ID_SIZE;
+    use crate::ctap::SignatureAlgorithm;
+    use crate::env::test::TestEnv;
+    use crypto::hmac::hmac_256;
+
+    const UNSUPPORTED_CREDENTIAL_ID_VERSION: u8 = 0x80;
+
+    fn test_encrypt_decrypt_credential(signature_algorithm: SignatureAlgorithm) {
+        let mut env = TestEnv::new();
+        let private_key = PrivateKey::new(&mut env, signature_algorithm);
+
+        let rp_id_hash = [0x55; 32];
+        let encrypted_id =
+            encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None).unwrap();
+        let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash, false)
+            .unwrap()
+            .unwrap();
+
+        assert_eq!(private_key, decrypted_source.private_key);
+    }
+
+    #[test]
+    fn test_encrypt_decrypt_ecdsa_credential() {
+        test_encrypt_decrypt_credential(SignatureAlgorithm::ES256);
+    }
+
+    #[test]
+    #[cfg(feature = "ed25519")]
+    fn test_encrypt_decrypt_ed25519_credential() {
+        test_encrypt_decrypt_credential(SignatureAlgorithm::EDDSA);
+    }
+
+    #[test]
+    fn test_encrypt_decrypt_bad_version() {
+        let mut env = TestEnv::new();
+        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
+
+        let rp_id_hash = [0x55; 32];
+        let mut encrypted_id =
+            encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None).unwrap();
+        encrypted_id[0] = UNSUPPORTED_CREDENTIAL_ID_VERSION;
+        // Override the HMAC to pass the check.
+        encrypted_id.truncate(&encrypted_id.len() - 32);
+        let hmac_key = env.key_store().key_handle_authentication().unwrap();
+        let id_hmac = hmac_256::<Sha256>(&hmac_key, &encrypted_id[..]);
+        encrypted_id.extend(&id_hmac);
+
+        assert_eq!(
+            decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash, false),
+            Ok(None)
+        );
+    }
+
+    fn test_encrypt_decrypt_bad_hmac(signature_algorithm: SignatureAlgorithm) {
+        let mut env = TestEnv::new();
+        let private_key = PrivateKey::new(&mut env, signature_algorithm);
+
+        let rp_id_hash = [0x55; 32];
+        let encrypted_id =
+            encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None).unwrap();
+        for i in 0..encrypted_id.len() {
+            let mut modified_id = encrypted_id.clone();
+            modified_id[i] ^= 0x01;
+            assert_eq!(
+                decrypt_credential_id(&mut env, modified_id, &rp_id_hash, false),
+                Ok(None)
+            );
+        }
+    }
+
+    #[test]
+    fn test_ecdsa_encrypt_decrypt_bad_hmac() {
+        test_encrypt_decrypt_bad_hmac(SignatureAlgorithm::ES256);
+    }
+
+    #[test]
+    #[cfg(feature = "ed25519")]
+    fn test_ed25519_encrypt_decrypt_bad_hmac() {
+        test_encrypt_decrypt_bad_hmac(SignatureAlgorithm::EDDSA);
+    }
+
+    fn test_decrypt_credential_missing_blocks(signature_algorithm: SignatureAlgorithm) {
+        let mut env = TestEnv::new();
+        let private_key = PrivateKey::new(&mut env, signature_algorithm);
+
+        let rp_id_hash = [0x55; 32];
+        let encrypted_id =
+            encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None).unwrap();
+
+        for length in (1..CBOR_CREDENTIAL_ID_SIZE).step_by(16) {
+            assert_eq!(
+                decrypt_credential_id(
+                    &mut env,
+                    encrypted_id[..length].to_vec(),
+                    &rp_id_hash,
+                    false
+                ),
+                Ok(None)
+            );
+        }
+    }
+
+    #[test]
+    fn test_ecdsa_decrypt_credential_missing_blocks() {
+        test_decrypt_credential_missing_blocks(SignatureAlgorithm::ES256);
+    }
+
+    #[test]
+    #[cfg(feature = "ed25519")]
+    fn test_ed25519_decrypt_credential_missing_blocks() {
+        test_decrypt_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 impl Env,
+        private_key: crypto::ecdsa::SecKey,
+        application: &[u8; 32],
+    ) -> Result<Vec<u8>, Ctap2StatusCode> {
+        let aes_enc_key =
+            crypto::aes256::EncryptionKey::new(&env.key_store().key_handle_encryption()?);
+        let mut plaintext = [0; 64];
+        private_key.to_bytes(array_mut_ref!(plaintext, 0, 32));
+        plaintext[32..64].copy_from_slice(application);
+
+        let mut encrypted_id = aes256_cbc_encrypt(env.rng(), &aes_enc_key, &plaintext, true)?;
+        let id_hmac = hmac_256::<Sha256>(
+            &env.key_store().key_handle_authentication()?,
+            &encrypted_id[..],
+        );
+        encrypted_id.extend(&id_hmac);
+        Ok(encrypted_id)
+    }
+
+    #[test]
+    fn test_encrypt_decrypt_credential_legacy() {
+        let mut env = TestEnv::new();
+        let private_key = PrivateKey::new_ecdsa(&mut env);
+        let ecdsa_key = private_key.ecdsa_key(&mut env).unwrap();
+
+        let rp_id_hash = [0x55; 32];
+        let encrypted_id =
+            legacy_encrypt_to_credential_id(&mut env, ecdsa_key, &rp_id_hash).unwrap();
+        // When checking credProtect for legacy credentials the check will always pass because we didn't persist credProtect
+        // policy info in it.
+        let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash, true)
+            .unwrap()
+            .unwrap();
+
+        assert_eq!(private_key, decrypted_source.private_key);
+    }
+
+    #[test]
+    fn test_encrypt_credential_size() {
+        let mut env = TestEnv::new();
+        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
+
+        let rp_id_hash = [0x55; 32];
+        let encrypted_id =
+            encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None).unwrap();
+        assert_eq!(encrypted_id.len(), CBOR_CREDENTIAL_ID_SIZE);
+    }
+
+    #[test]
+    fn test_check_cred_protect_fail() {
+        let mut env = TestEnv::new();
+        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
+
+        let rp_id_hash = [0x55; 32];
+        let encrypted_id = encrypt_to_credential_id(
+            &mut env,
+            &private_key,
+            &rp_id_hash,
+            Some(CredentialProtectionPolicy::UserVerificationRequired),
+        )
+        .unwrap();
+
+        assert_eq!(
+            decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash, true),
+            Ok(None)
+        );
+    }
+
+    #[test]
+    fn test_check_cred_protect_success() {
+        let mut env = TestEnv::new();
+        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
+
+        let rp_id_hash = [0x55; 32];
+        let encrypted_id = encrypt_to_credential_id(
+            &mut env,
+            &private_key,
+            &rp_id_hash,
+            Some(CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList),
+        )
+        .unwrap();
+
+        let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash, true)
+            .unwrap()
+            .unwrap();
+
+        assert_eq!(decrypted_source.private_key, private_key);
+        assert_eq!(
+            decrypted_source.cred_protect_policy,
+            Some(CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList)
+        );
+    }
+}

src/ctap/crypto_wrapper.rs

@@ -13,43 +13,19 @@
 // limitations under the License.
 
 use crate::api::key_store::KeyStore;
-#[cfg(feature = "ed25519")]
+use crate::ctap::data_formats::{extract_array, extract_byte_string, CoseKey, SignatureAlgorithm};
-use crate::ctap::data_formats::EDDSA_ALGORITHM;
-use crate::ctap::data_formats::{
-    extract_array, extract_byte_string, CoseKey, PublicKeyCredentialSource,
-    PublicKeyCredentialType, SignatureAlgorithm, ES256_ALGORITHM,
-};
 use crate::ctap::status_code::Ctap2StatusCode;
 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;
-
-const ECDSA_CREDENTIAL_ID_VERSION: u8 = 0x01;
-#[allow(dead_code)]
-const ED25519_CREDENTIAL_ID_VERSION: u8 = 0x02;
-#[cfg(test)]
-const UNSUPPORTED_CREDENTIAL_ID_VERSION: u8 = 0x80;
-
 /// Wraps the AES256-CBC encryption to match what we need in CTAP.
 pub fn aes256_cbc_encrypt(
     rng: &mut dyn Rng256,
@@ -214,8 +190,8 @@ fn ecdsa_key_from_seed(
     Ok(ecdsa::SecKey::from_bytes(&ecdsa_bytes).unwrap())
 }
 
-impl From<PrivateKey> for cbor::Value {
+impl From<&PrivateKey> for cbor::Value {
-    fn from(private_key: PrivateKey) -> Self {
+    fn from(private_key: &PrivateKey) -> Self {
         cbor_array![
             cbor_int!(private_key.signature_algorithm() as i64),
             cbor_bytes!(private_key.to_bytes()),
@@ -243,133 +219,6 @@ impl TryFrom<cbor::Value> for PrivateKey {
     }
 }
 
-/// 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.
-///
-/// For v1 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.
-///
-/// For v2 we write the following data for EdDSA over curve Ed25519 (algorithm -8, curve 6):
-/// -  1 byte : version number
-/// - 16 bytes: initialization vector for AES-256,
-/// - 32 bytes: Ed25519 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 aes_enc_key = crypto::aes256::EncryptionKey::new(&env.key_store().key_handle_encryption()?);
-
-    let mut plaintext = [0; 64];
-    let version = match private_key {
-        PrivateKey::Ecdsa(ecdsa_seed) => {
-            plaintext[..32].copy_from_slice(ecdsa_seed);
-            ECDSA_CREDENTIAL_ID_VERSION
-        }
-        #[cfg(feature = "ed25519")]
-        PrivateKey::Ed25519(ed25519_key) => {
-            let sk_bytes = *ed25519_key.seed();
-            plaintext[0..32].copy_from_slice(&sk_bytes);
-            ED25519_CREDENTIAL_ID_VERSION
-        }
-    };
-    plaintext[32..64].copy_from_slice(application);
-    let mut encrypted_id = aes256_cbc_encrypt(env.rng(), &aes_enc_key, &plaintext, true)?;
-    encrypted_id.insert(0, version);
-
-    let id_hmac = hmac_256::<Sha256>(
-        &env.key_store().key_handle_authentication()?,
-        &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)
-/// - v2 (EdDSA over curve Ed25519)
-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 hmac_message_size = credential_id.len() - 32;
-    if !verify_hmac_256::<Sha256>(
-        &env.key_store().key_handle_authentication()?,
-        &credential_id[..hmac_message_size],
-        array_ref![credential_id, hmac_message_size, 32],
-    ) {
-        return Ok(None);
-    }
-
-    let (payload, algorithm) = if credential_id.len() == LEGACY_CREDENTIAL_ID_SIZE {
-        (&credential_id[..hmac_message_size], ES256_ALGORITHM)
-    } else {
-        // Version number check
-        let algorithm = match credential_id[0] {
-            ECDSA_CREDENTIAL_ID_VERSION => ES256_ALGORITHM,
-            #[cfg(feature = "ed25519")]
-            ED25519_CREDENTIAL_ID_VERSION => EDDSA_ALGORITHM,
-            _ => return Ok(None),
-        };
-        (&credential_id[1..hmac_message_size], algorithm)
-    };
-    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(&env.key_store().key_handle_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 = match algorithm {
-        ES256_ALGORITHM => PrivateKey::new_ecdsa_from_bytes(&decrypted_id[..32]),
-        #[cfg(feature = "ed25519")]
-        EDDSA_ALGORITHM => PrivateKey::new_ed25519_from_bytes(&decrypted_id[..32]),
-        _ => return Ok(None),
-    };
-
-    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::*;
@@ -525,7 +374,7 @@ mod test {
     fn test_private_key_from_to_cbor(signature_algorithm: SignatureAlgorithm) {
         let mut env = TestEnv::new();
         let private_key = PrivateKey::new(&mut env, signature_algorithm);
-        let cbor = cbor::Value::from(private_key.clone());
+        let cbor = cbor::Value::from(&private_key);
         assert_eq!(PrivateKey::try_from(cbor), Ok(private_key),);
     }
 
@@ -576,147 +425,4 @@ mod test {
             Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR),
         );
     }
-
-    fn test_encrypt_decrypt_credential(signature_algorithm: SignatureAlgorithm) {
-        let mut env = TestEnv::new();
-        let private_key = PrivateKey::new(&mut env, signature_algorithm);
-
-        let rp_id_hash = [0x55; 32];
-        let encrypted_id = encrypt_key_handle(&mut env, &private_key, &rp_id_hash).unwrap();
-        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_ecdsa_credential() {
-        test_encrypt_decrypt_credential(SignatureAlgorithm::ES256);
-    }
-
-    #[test]
-    #[cfg(feature = "ed25519")]
-    fn test_encrypt_decrypt_ed25519_credential() {
-        test_encrypt_decrypt_credential(SignatureAlgorithm::EDDSA);
-    }
-
-    #[test]
-    fn test_encrypt_decrypt_bad_version() {
-        let mut env = TestEnv::new();
-        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
-
-        let rp_id_hash = [0x55; 32];
-        let mut encrypted_id = encrypt_key_handle(&mut env, &private_key, &rp_id_hash).unwrap();
-        encrypted_id[0] = UNSUPPORTED_CREDENTIAL_ID_VERSION;
-        // Override the HMAC to pass the check.
-        encrypted_id.truncate(&encrypted_id.len() - 32);
-        let hmac_key = env.key_store().key_handle_authentication().unwrap();
-        let id_hmac = hmac_256::<Sha256>(&hmac_key, &encrypted_id[..]);
-        encrypted_id.extend(&id_hmac);
-
-        assert_eq!(
-            decrypt_credential_source(&mut env, encrypted_id, &rp_id_hash),
-            Ok(None)
-        );
-    }
-
-    fn test_encrypt_decrypt_bad_hmac(signature_algorithm: SignatureAlgorithm) {
-        let mut env = TestEnv::new();
-        let private_key = PrivateKey::new(&mut env, signature_algorithm);
-
-        let rp_id_hash = [0x55; 32];
-        let encrypted_id = encrypt_key_handle(&mut env, &private_key, &rp_id_hash).unwrap();
-        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_ecdsa_encrypt_decrypt_bad_hmac() {
-        test_encrypt_decrypt_bad_hmac(SignatureAlgorithm::ES256);
-    }
-
-    #[test]
-    #[cfg(feature = "ed25519")]
-    fn test_ed25519_encrypt_decrypt_bad_hmac() {
-        test_encrypt_decrypt_bad_hmac(SignatureAlgorithm::EDDSA);
-    }
-
-    fn test_decrypt_credential_missing_blocks(signature_algorithm: SignatureAlgorithm) {
-        let mut env = TestEnv::new();
-        let private_key = PrivateKey::new(&mut env, signature_algorithm);
-
-        let rp_id_hash = [0x55; 32];
-        let encrypted_id = encrypt_key_handle(&mut env, &private_key, &rp_id_hash).unwrap();
-
-        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)
-            );
-        }
-    }
-
-    #[test]
-    fn test_ecdsa_decrypt_credential_missing_blocks() {
-        test_decrypt_credential_missing_blocks(SignatureAlgorithm::ES256);
-    }
-
-    #[test]
-    #[cfg(feature = "ed25519")]
-    fn test_ed25519_decrypt_credential_missing_blocks() {
-        test_decrypt_credential_missing_blocks(SignatureAlgorithm::EDDSA);
-    }
-
-    /// 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 aes_enc_key =
-            crypto::aes256::EncryptionKey::new(&env.key_store().key_handle_encryption()?);
-        let mut plaintext = [0; 64];
-        private_key.to_bytes(array_mut_ref!(plaintext, 0, 32));
-        plaintext[32..64].copy_from_slice(application);
-
-        let mut encrypted_id = aes256_cbc_encrypt(env.rng(), &aes_enc_key, &plaintext, true)?;
-        let id_hmac = hmac_256::<Sha256>(
-            &env.key_store().key_handle_authentication()?,
-            &encrypted_id[..],
-        );
-        encrypted_id.extend(&id_hmac);
-        Ok(encrypted_id)
-    }
-
-    #[test]
-    fn test_encrypt_decrypt_credential_legacy() {
-        let mut env = TestEnv::new();
-        let private_key = PrivateKey::new_ecdsa(&mut env);
-        let ecdsa_key = private_key.ecdsa_key(&mut env).unwrap();
-
-        let rp_id_hash = [0x55; 32];
-        let encrypted_id = legacy_encrypt_key_handle(&mut env, ecdsa_key, &rp_id_hash).unwrap();
-        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();
-        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::ES256);
-
-        let rp_id_hash = [0x55; 32];
-        let encrypted_id = encrypt_key_handle(&mut env, &private_key, &rp_id_hash).unwrap();
-        assert_eq!(encrypted_id.len(), ECDSA_CREDENTIAL_ID_SIZE);
-    }
 }

src/ctap/ctap1.rs

@@ -14,7 +14,8 @@
 
 use super::super::clock::CtapInstant;
 use super::apdu::{Apdu, ApduStatusCode};
-use super::crypto_wrapper::{decrypt_credential_source, encrypt_key_handle, PrivateKey};
+use super::credential_id::{decrypt_credential_id, encrypt_to_credential_id};
+use super::crypto_wrapper::PrivateKey;
 use super::CtapState;
 use crate::ctap::storage;
 use crate::env::Env;
@@ -250,7 +251,7 @@ impl Ctap1Command {
             .ecdsa_key(env)
             .map_err(|_| Ctap1StatusCode::SW_INTERNAL_EXCEPTION)?;
         let pk = sk.genpk();
-        let key_handle = encrypt_key_handle(env, &private_key, &application)
+        let key_handle = encrypt_to_credential_id(env, &private_key, &application, None)
             .map_err(|_| Ctap1StatusCode::SW_INTERNAL_EXCEPTION)?;
         if key_handle.len() > 0xFF {
             // This is just being defensive with unreachable code.
@@ -309,7 +310,7 @@ impl Ctap1Command {
         flags: Ctap1Flags,
         ctap_state: &mut CtapState,
     ) -> Result<Vec<u8>, Ctap1StatusCode> {
-        let credential_source = decrypt_credential_source(env, key_handle, &application)
+        let credential_source = decrypt_credential_id(env, key_handle, &application, false)
             .map_err(|_| Ctap1StatusCode::SW_WRONG_DATA)?;
         if let Some(credential_source) = credential_source {
             let ecdsa_key = credential_source
@@ -343,7 +344,7 @@ impl Ctap1Command {
 
 #[cfg(test)]
 mod test {
-    use super::super::crypto_wrapper::ECDSA_CREDENTIAL_ID_SIZE;
+    use super::super::credential_id::CBOR_CREDENTIAL_ID_SIZE;
     use super::super::data_formats::SignatureAlgorithm;
     use super::super::key_material;
     use super::*;
@@ -379,13 +380,12 @@ mod test {
             flags.into(),
             0x00,
             0x00,
-            0x00,
-            65 + ECDSA_CREDENTIAL_ID_SIZE as u8,
         ];
+        message.extend(&(65 + CBOR_CREDENTIAL_ID_SIZE as u16).to_be_bytes());
         let challenge = [0x0C; 32];
         message.extend(&challenge);
         message.extend(application);
-        message.push(ECDSA_CREDENTIAL_ID_SIZE as u8);
+        message.push(CBOR_CREDENTIAL_ID_SIZE as u8);
         message.extend(key_handle);
         message
     }
@@ -444,15 +444,16 @@ 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], ECDSA_CREDENTIAL_ID_SIZE as u8);
+        assert_eq!(response[66], CBOR_CREDENTIAL_ID_SIZE as u8);
-        assert!(decrypt_credential_source(
+        assert!(decrypt_credential_id(
             &mut env,
-            response[67..67 + ECDSA_CREDENTIAL_ID_SIZE].to_vec(),
+            response[67..67 + CBOR_CREDENTIAL_ID_SIZE].to_vec(),
-            &application
+            &application,
+            false
         )
         .unwrap()
         .is_some());
-        const CERT_START: usize = 67 + ECDSA_CREDENTIAL_ID_SIZE;
+        const CERT_START: usize = 67 + CBOR_CREDENTIAL_ID_SIZE;
         assert_eq!(
             &response[CERT_START..CERT_START + fake_cert.len()],
             &fake_cert[..]
@@ -507,7 +508,7 @@ mod test {
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = encrypt_key_handle(&mut env, &sk, &application).unwrap();
+        let key_handle = encrypt_to_credential_id(&mut env, &sk, &application, None).unwrap();
         let message = create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle);
 
         let response =
@@ -525,7 +526,7 @@ mod test {
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = encrypt_key_handle(&mut env, &sk, &application).unwrap();
+        let key_handle = encrypt_to_credential_id(&mut env, &sk, &application, None).unwrap();
         let application = [0x55; 32];
         let message = create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle);
 
@@ -544,7 +545,7 @@ mod test {
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = encrypt_key_handle(&mut env, &sk, &application).unwrap();
+        let key_handle = encrypt_to_credential_id(&mut env, &sk, &application, None).unwrap();
         let mut message = create_authenticate_message(
             &application,
             Ctap1Flags::DontEnforceUpAndSign,
@@ -582,7 +583,7 @@ mod test {
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = encrypt_key_handle(&mut env, &sk, &application).unwrap();
+        let key_handle = encrypt_to_credential_id(&mut env, &sk, &application, None).unwrap();
         let mut message =
             create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle);
         message[0] = 0xEE;
@@ -602,7 +603,7 @@ mod test {
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = encrypt_key_handle(&mut env, &sk, &application).unwrap();
+        let key_handle = encrypt_to_credential_id(&mut env, &sk, &application, None).unwrap();
         let mut message =
             create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle);
         message[1] = 0xEE;
@@ -622,7 +623,7 @@ mod test {
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = encrypt_key_handle(&mut env, &sk, &application).unwrap();
+        let key_handle = encrypt_to_credential_id(&mut env, &sk, &application, None).unwrap();
         let mut message =
             create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle);
         message[2] = 0xEE;
@@ -650,7 +651,7 @@ mod test {
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = encrypt_key_handle(&mut env, &sk, &application).unwrap();
+        let key_handle = encrypt_to_credential_id(&mut env, &sk, &application, None).unwrap();
         let message =
             create_authenticate_message(&application, Ctap1Flags::EnforceUpAndSign, &key_handle);
 
@@ -678,7 +679,7 @@ mod test {
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = encrypt_key_handle(&mut env, &sk, &application).unwrap();
+        let key_handle = encrypt_to_credential_id(&mut env, &sk, &application, None).unwrap();
         let message = create_authenticate_message(
             &application,
             Ctap1Flags::DontEnforceUpAndSign,
@@ -704,7 +705,7 @@ mod test {
     #[test]
     fn test_process_authenticate_bad_key_handle() {
         let application = [0x0A; 32];
-        let key_handle = vec![0x00; ECDSA_CREDENTIAL_ID_SIZE];
+        let key_handle = vec![0x00; CBOR_CREDENTIAL_ID_SIZE];
         let message =
             create_authenticate_message(&application, Ctap1Flags::EnforceUpAndSign, &key_handle);
 
@@ -723,7 +724,7 @@ mod test {
     #[test]
     fn test_process_authenticate_without_up() {
         let application = [0x0A; 32];
-        let key_handle = vec![0x00; ECDSA_CREDENTIAL_ID_SIZE];
+        let key_handle = vec![0x00; CBOR_CREDENTIAL_ID_SIZE];
         let message =
             create_authenticate_message(&application, Ctap1Flags::EnforceUpAndSign, &key_handle);
 

src/ctap/mod.rs

@@ -16,6 +16,7 @@ pub mod apdu;
 mod client_pin;
 pub mod command;
 mod config_command;
+mod credential_id;
 mod credential_management;
 mod crypto_wrapper;
 #[cfg(feature = "with_ctap1")]
@@ -40,10 +41,11 @@ use self::command::{
     AuthenticatorVendorConfigureParameters, AuthenticatorVendorUpgradeParameters, Command,
 };
 use self::config_command::process_config;
-use self::credential_management::process_credential_management;
+use self::credential_id::{
-use self::crypto_wrapper::{
+    decrypt_credential_id, encrypt_to_credential_id, MAX_CREDENTIAL_ID_SIZE,
-    decrypt_credential_source, encrypt_key_handle, PrivateKey, MAX_CREDENTIAL_ID_SIZE,
 };
+use self::credential_management::process_credential_management;
+use self::crypto_wrapper::PrivateKey;
 use self::data_formats::{
     AuthenticatorTransport, CoseKey, CoseSignature, CredentialProtectionPolicy,
     EnterpriseAttestationMode, GetAssertionExtensions, PackedAttestationStatement,
@@ -807,7 +809,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()
-                    || decrypt_credential_source(env, cred_desc.key_id, &rp_id_hash)?.is_some()
+                    || decrypt_credential_id(env, cred_desc.key_id, &rp_id_hash, !has_uv)?.is_some()
                 {
                     // Perform this check, so bad actors can't brute force exclude_list
                     // without user interaction.
@@ -881,7 +883,7 @@ impl CtapState {
             storage::store_credential(env, credential_source)?;
             random_id
         } else {
-            encrypt_key_handle(env, &private_key, &rp_id_hash)?
+            encrypt_to_credential_id(env, &private_key, &rp_id_hash, cred_protect_policy)?
         };
 
         let mut auth_data = self.generate_auth_data(env, &rp_id_hash, flags)?;
@@ -1070,7 +1072,8 @@ impl CtapState {
             if credential.is_some() {
                 return Ok(credential);
             }
-            let credential = decrypt_credential_source(env, allowed_credential.key_id, rp_id_hash)?;
+            let credential =
+                decrypt_credential_id(env, allowed_credential.key_id, rp_id_hash, !has_uv)?;
             if credential.is_some() {
                 return Ok(credential);
             }
@@ -1491,7 +1494,7 @@ mod test {
         AuthenticatorAttestationMaterial, AuthenticatorClientPinParameters,
         AuthenticatorCredentialManagementParameters,
     };
-    use super::crypto_wrapper::ECDSA_CREDENTIAL_ID_SIZE;
+    use super::credential_id::CBOR_CREDENTIAL_ID_SIZE;
     use super::data_formats::{
         ClientPinSubCommand, CoseKey, CredentialManagementSubCommand, GetAssertionHmacSecretInput,
         GetAssertionOptions, MakeCredentialExtensions, MakeCredentialOptions, PinUvAuthProtocol,
@@ -1698,7 +1701,7 @@ mod test {
             make_credential_response,
             0x41,
             &storage::aaguid(&mut env).unwrap(),
-            ECDSA_CREDENTIAL_ID_SIZE as u8,
+            CBOR_CREDENTIAL_ID_SIZE as u8,
             &[],
         );
     }
@@ -1825,7 +1828,7 @@ mod test {
             make_credential_response,
             0xC1,
             &storage::aaguid(&mut env).unwrap(),
-            ECDSA_CREDENTIAL_ID_SIZE as u8,
+            CBOR_CREDENTIAL_ID_SIZE as u8,
             &expected_extension_cbor,
         );
     }
@@ -2068,7 +2071,7 @@ mod test {
             make_credential_response,
             0x41,
             &storage::aaguid(&mut env).unwrap(),
-            ECDSA_CREDENTIAL_ID_SIZE as u8,
+            CBOR_CREDENTIAL_ID_SIZE as u8,
             &[],
         );
     }
@@ -2436,8 +2439,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, ECDSA_CREDENTIAL_ID_SIZE);
+                assert_eq!(auth_data[offset + 1] as usize, CBOR_CREDENTIAL_ID_SIZE);
-                auth_data[offset + 2..offset + 2 + ECDSA_CREDENTIAL_ID_SIZE].to_vec()
+                auth_data[offset + 2..offset + 2 + CBOR_CREDENTIAL_ID_SIZE].to_vec()
             }
             _ => panic!("Invalid response type"),
         };