Persist/parse slot_id in/from credential (#569)

* Persist/parse slot_id in/from credential

Persist slot_id into credential_id or the resident credential record
during MakeCredential, and parse it during GetAssertion. Add related
unittests.

* Fix styles

* Move enable_pin_uv back to ctap/mod.rs

src/ctap/credential_id.rs

@@ -19,7 +19,7 @@ use super::data_formats::{
 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::ctap::data_formats::{extract_byte_string, extract_map, extract_unsigned};
 use crate::env::Env;
 use alloc::string::String;
 use alloc::vec::Vec;
@@ -48,6 +48,7 @@ struct CredentialSource {
     rp_id_hash: [u8; 32],
     cred_protect_policy: Option<CredentialProtectionPolicy>,
     cred_blob: Option<Vec<u8>>,
+    slot_id: Option<usize>,
 }
 
 // The data fields contained in the credential ID are serialized using CBOR maps.
@@ -57,6 +58,7 @@ enum CredentialSourceField {
     RpIdHash = 1,
     CredProtectPolicy = 2,
     CredBlob = 3,
+    SlotId = 4,
 }
 
 impl From<CredentialSourceField> for sk_cbor::Value {
@@ -84,6 +86,7 @@ fn decrypt_legacy_credential_id(
         rp_id_hash: plaintext[32..64].try_into().unwrap(),
         cred_protect_policy: None,
         cred_blob: None,
+        slot_id: None,
     }))
 }
 
@@ -102,6 +105,7 @@ fn decrypt_cbor_credential_id(
           CredentialSourceField::RpIdHash=> rp_id_hash,
           CredentialSourceField::CredProtectPolicy => cred_protect_policy,
           CredentialSourceField::CredBlob => cred_blob,
+          CredentialSourceField::SlotId => slot_id,
       } = extract_map(cbor_credential_source)?;
     }
     Ok(match (private_key, rp_id_hash) {
@@ -115,11 +119,19 @@ fn decrypt_cbor_credential_id(
                 .map(CredentialProtectionPolicy::try_from)
                 .transpose()?;
             let cred_blob = cred_blob.map(extract_byte_string).transpose()?;
+            let slot_id = match slot_id.map(extract_unsigned).transpose()? {
+                Some(x) => Some(
+                    usize::try_from(x)
+                        .map_err(|_| Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?,
+                ),
+                None => None,
+            };
             Some(CredentialSource {
                 private_key,
                 rp_id_hash: rp_id_hash.try_into().unwrap(),
                 cred_protect_policy,
                 cred_blob,
+                slot_id,
             })
         }
         _ => None,
@@ -167,13 +179,15 @@ pub fn encrypt_to_credential_id(
     rp_id_hash: &[u8; 32],
     cred_protect_policy: Option<CredentialProtectionPolicy>,
     cred_blob: Option<Vec<u8>>,
+    slot_id: usize,
 ) -> 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,
       CredentialSourceField::CredBlob => cred_blob,
+      CredentialSourceField::SlotId => slot_id as u64,
     };
     cbor_write(cbor, &mut payload)?;
     add_padding(&mut payload)?;
@@ -262,6 +276,7 @@ pub fn decrypt_credential_id(
         user_icon: None,
         cred_blob: credential_source.cred_blob,
         large_blob_key: None,
+        slot_id: credential_source.slot_id,
     }))
 }
 
@@ -282,7 +297,7 @@ mod test {
 
         let rp_id_hash = [0x55; 32];
         let encrypted_id =
-            encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None, None).unwrap();
+            encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None, None, 0).unwrap();
         let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash)
             .unwrap()
             .unwrap();
@@ -308,7 +323,7 @@ mod test {
 
         let rp_id_hash = [0x55; 32];
         let mut encrypted_id =
-            encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None, None).unwrap();
+            encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None, None, 0).unwrap();
         encrypted_id[0] = UNSUPPORTED_CREDENTIAL_ID_VERSION;
         // Override the HMAC to pass the check.
         encrypted_id.truncate(&encrypted_id.len() - 32);
@@ -328,7 +343,7 @@ mod test {
 
         let rp_id_hash = [0x55; 32];
         let encrypted_id =
-            encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None, None).unwrap();
+            encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None, None, 0).unwrap();
         for i in 0..encrypted_id.len() {
             let mut modified_id = encrypted_id.clone();
             modified_id[i] ^= 0x01;
@@ -356,7 +371,7 @@ mod test {
 
         let rp_id_hash = [0x55; 32];
         let encrypted_id =
-            encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None, None).unwrap();
+            encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None, None, 0).unwrap();
 
         for length in (1..CBOR_CREDENTIAL_ID_SIZE).step_by(16) {
             assert_eq!(
@@ -423,7 +438,7 @@ mod test {
 
         let rp_id_hash = [0x55; 32];
         let encrypted_id =
-            encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None, None).unwrap();
+            encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None, None, 0).unwrap();
         assert_eq!(encrypted_id.len(), CBOR_CREDENTIAL_ID_SIZE);
     }
 
@@ -444,6 +459,7 @@ mod test {
             &rp_id_hash,
             cred_protect_policy,
             cred_blob,
+            0,
         );
 
         assert!(encrypted_id.is_ok());
@@ -461,6 +477,7 @@ mod test {
             &rp_id_hash,
             Some(CredentialProtectionPolicy::UserVerificationRequired),
             None,
+            0,
         )
         .unwrap();
 
@@ -481,14 +498,38 @@ mod test {
 
         let rp_id_hash = [0x55; 32];
         let cred_blob = Some(vec![0x55; env.customization().max_cred_blob_length()]);
+        let encrypted_id = encrypt_to_credential_id(
+            &mut env,
+            &private_key,
+            &rp_id_hash,
+            None,
+            cred_blob.clone(),
+            0,
+        )
+        .unwrap();
+
+        let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash)
+            .unwrap()
+            .unwrap();
+        assert_eq!(decrypted_source.private_key, private_key);
+        assert_eq!(decrypted_source.cred_blob, cred_blob);
+    }
+
+    #[test]
+    fn test_slot_id_persisted() {
+        let mut env = TestEnv::new();
+        let private_key = PrivateKey::new(&mut env, SignatureAlgorithm::Es256);
+
+        let rp_id_hash = [0x55; 32];
+        let slot_id = 1;
         let encrypted_id =
-            encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None, cred_blob.clone())
+            encrypt_to_credential_id(&mut env, &private_key, &rp_id_hash, None, None, slot_id)
                 .unwrap();
 
         let decrypted_source = decrypt_credential_id(&mut env, encrypted_id, &rp_id_hash)
             .unwrap()
             .unwrap();
         assert_eq!(decrypted_source.private_key, private_key);
-        assert_eq!(decrypted_source.cred_blob, cred_blob);
+        assert_eq!(decrypted_source.slot_id, Some(slot_id));
     }
 }

src/ctap/credential_management.rs

@@ -81,6 +81,7 @@ fn enumerate_credentials_response(
         user_icon,
         cred_blob: _,
         large_blob_key,
+        slot_id: _,
     } = credential;
     let user = PublicKeyCredentialUserEntity {
         user_id: user_handle,
@@ -183,12 +184,17 @@ fn process_enumerate_credentials_begin(
         .rp_id_hash
         .ok_or(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER)?;
     client_pin.has_no_or_rp_id_hash_permission(&rp_id_hash[..])?;
+    // enumerateCredentials needs UV, so slot_id must not be None.
+    let slot_id = client_pin
+        .get_slot_id_in_use_or_default(env)?
+        .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?;
     let mut iter_result = Ok(());
     let iter = storage::iter_credentials(env, &mut iter_result)?;
     let mut rp_credentials: Vec<usize> = iter
         .filter_map(|(key, credential)| {
             let cred_rp_id_hash = Sha256::hash(credential.rp_id.as_bytes());
-            if cred_rp_id_hash == rp_id_hash.as_slice() {
+            let slot_id_matches = credential.slot_id.unwrap_or(0) == slot_id;
+            if cred_rp_id_hash == rp_id_hash.as_slice() && slot_id_matches {
                 Some(key)
             } else {
                 None
@@ -385,6 +391,7 @@ mod test {
             user_icon: Some("icon".to_string()),
             cred_blob: None,
             large_blob_key: None,
+            slot_id: None,
         }
     }
 
@@ -766,6 +773,120 @@ mod test {
         );
     }
 
+    #[test]
+    fn test_process_enumerate_credentials_multi_pin() {
+        let mut env = TestEnv::new();
+        storage::_enable_multi_pin_for_test(&mut env).unwrap();
+        let key_agreement_key = crypto::ecdh::SecKey::gensk(env.rng());
+        let pin_uv_auth_token = [0x55; 32];
+        let client_pin = ClientPin::new_test(
+            &mut env,
+            1,
+            key_agreement_key,
+            pin_uv_auth_token,
+            PinUvAuthProtocol::V1,
+        );
+
+        // credential_source1 has no slot_id, so should be treated as slot 0. Only credential_source 2 and 4
+        // should be discovered.
+        let credential_source1 = create_credential_source(&mut env);
+        let mut credential_source2 = create_credential_source(&mut env);
+        credential_source2.user_handle = vec![0x02];
+        credential_source2.slot_id = Some(1);
+        let mut credential_source3 = create_credential_source(&mut env);
+        credential_source3.user_handle = vec![0x03];
+        credential_source3.slot_id = Some(2);
+        let mut credential_source4 = create_credential_source(&mut env);
+        credential_source4.user_handle = vec![0x04];
+        credential_source4.slot_id = Some(1);
+
+        let mut ctap_state = CtapState::new(&mut env, CtapInstant::new(0));
+        ctap_state.client_pin = client_pin;
+
+        storage::store_credential(&mut env, credential_source1).unwrap();
+        storage::store_credential(&mut env, credential_source2).unwrap();
+        storage::store_credential(&mut env, credential_source3).unwrap();
+        storage::store_credential(&mut env, credential_source4).unwrap();
+
+        storage::set_pin(&mut env, 1, &[0u8; 16], 4).unwrap();
+        let pin_uv_auth_param = Some(vec![
+            0xF8, 0xB0, 0x3C, 0xC1, 0xD5, 0x58, 0x9C, 0xB7, 0x4D, 0x42, 0xA1, 0x64, 0x14, 0x28,
+            0x2B, 0x68,
+        ]);
+
+        let sub_command_params = CredentialManagementSubCommandParameters {
+            rp_id_hash: Some(Sha256::hash(b"example.com").to_vec()),
+            credential_id: None,
+            user: None,
+        };
+        // RP ID hash:
+        // A379A6F6EEAFB9A55E378C118034E2751E682FAB9F2D30AB13D2125586CE1947
+        let cred_management_params = AuthenticatorCredentialManagementParameters {
+            sub_command: CredentialManagementSubCommand::EnumerateCredentialsBegin,
+            sub_command_params: Some(sub_command_params),
+            pin_uv_auth_protocol: Some(PinUvAuthProtocol::V1),
+            pin_uv_auth_param,
+        };
+        let cred_management_response = process_credential_management(
+            &mut env,
+            &mut ctap_state.stateful_command_permission,
+            &mut ctap_state.client_pin,
+            cred_management_params,
+            DUMMY_CHANNEL,
+            CtapInstant::new(0),
+        );
+        match cred_management_response.unwrap() {
+            ResponseData::AuthenticatorCredentialManagement(Some(response)) => {
+                assert!(response.user.is_some());
+                assert!(response.public_key.is_some());
+                assert_eq!(response.total_credentials, Some(2));
+            }
+            _ => panic!("Invalid response type"),
+        };
+
+        let cred_management_params = AuthenticatorCredentialManagementParameters {
+            sub_command: CredentialManagementSubCommand::EnumerateCredentialsGetNextCredential,
+            sub_command_params: None,
+            pin_uv_auth_protocol: None,
+            pin_uv_auth_param: None,
+        };
+        let cred_management_response = process_credential_management(
+            &mut env,
+            &mut ctap_state.stateful_command_permission,
+            &mut ctap_state.client_pin,
+            cred_management_params,
+            DUMMY_CHANNEL,
+            CtapInstant::new(0),
+        );
+        match cred_management_response.unwrap() {
+            ResponseData::AuthenticatorCredentialManagement(Some(response)) => {
+                assert!(response.user.is_some());
+                assert!(response.public_key.is_some());
+                assert_eq!(response.total_credentials, None);
+            }
+            _ => panic!("Invalid response type"),
+        };
+
+        let cred_management_params = AuthenticatorCredentialManagementParameters {
+            sub_command: CredentialManagementSubCommand::EnumerateCredentialsGetNextCredential,
+            sub_command_params: None,
+            pin_uv_auth_protocol: None,
+            pin_uv_auth_param: None,
+        };
+        let cred_management_response = process_credential_management(
+            &mut env,
+            &mut ctap_state.stateful_command_permission,
+            &mut ctap_state.client_pin,
+            cred_management_params,
+            DUMMY_CHANNEL,
+            CtapInstant::new(0),
+        );
+        assert_eq!(
+            cred_management_response,
+            Err(Ctap2StatusCode::CTAP2_ERR_NOT_ALLOWED)
+        );
+    }
+
     #[test]
     fn test_process_delete_credential() {
         let mut env = TestEnv::new();