Repository: google/OpenSK
File: src/ctap/storage.rs (Rust)

Code before:
    pub fn _delete_credential(&mut self, credential_id: &[u8]) -> Result<(), Ctap2StatusCode> {
        let (key, _) = self.find_credential_item(credential_id)?;
        Ok(self.store.remove(key)?)
    }

    /// Updates a credential's user information.
    ///
    /// # Errors
    ///
    /// Returns `CTAP2_ERR_NO_CREDENTIALS` if the credential is not found.
    pub fn _update_credential(
        &mut self,
        credential_id: &[u8],
        user: PublicKeyCredentialUserEntity,
    ) -> Result<(), Ctap2StatusCode> {
        let (key, mut credential) = self.find_credential_item(credential_id)?;
        credential.user_name = user.user_name;
        credential.user_display_name = user.user_display_name;
        credential.user_icon = user.user_icon;
        let value = serialize_credential(credential)?;
        Ok(self.store.insert(key, &value)?)
    }

    /// Returns the list of matching credentials.
    ///
    /// Does not return credentials that are not discoverable if `check_cred_protect` is set.
    pub fn filter_credentials(
        &self,
        rp_id: &str,
        check_cred_protect: bool,
    ) -> Result<Vec<(usize, u64)>, Ctap2StatusCode> {
        let mut iter_result = Ok(());
        let iter = self.iter_credentials(&mut iter_result)?;
        let result = iter
            .filter_map(|(key, credential)| {
                if credential.rp_id == rp_id
                    && (!check_cred_protect || credential.is_discoverable())
                {
                    Some((key, credential.creation_order))
                } else {
                    None
                }
            })
            .collect();
        iter_result?;
        Ok(result)
    }

    /// Returns the number of credentials.
    pub fn count_credentials(&self) -> Result<usize, Ctap2StatusCode> {
        let mut iter_result = Ok(());

Review comment (security):
Let's update the documentation to say that the function returns pairs of key and creation order. But this is a bit weird since it looks quite custom logic. Shouldn't we inline this logic in the only place where `filter_credentials` is used?

Code after:
    pub fn _delete_credential(&mut self, credential_id: &[u8]) -> Result<(), Ctap2StatusCode> {
        let (key, _) = self.find_credential_item(credential_id)?;
        Ok(self.store.remove(key)?)
    }

    /// Updates a credential's user information.
    ///
    /// # Errors
    ///
    /// Returns `CTAP2_ERR_NO_CREDENTIALS` if the credential is not found.
    pub fn _update_credential(
        &mut self,
        credential_id: &[u8],
        user: PublicKeyCredentialUserEntity,
    ) -> Result<(), Ctap2StatusCode> {
        let (key, mut credential) = self.find_credential_item(credential_id)?;
        credential.user_name = user.user_name;
        credential.user_display_name = user.user_display_name;
        credential.user_icon = user.user_icon;
        let value = serialize_credential(credential)?;
        Ok(self.store.insert(key, &value)?)
    }

    /// Returns the number of credentials.
    pub fn count_credentials(&self) -> Result<usize, Ctap2StatusCode> {
        let mut iter_result = Ok(());
        let iter = self.iter_credentials(&mut iter_result)?;
        let result = iter.count();
        iter_result?;
        Ok(result)
    }

    /// Returns the estimated number of credentials that can still be stored.
    pub fn remaining_credentials(&self) -> Result<usize, Ctap2StatusCode> {
        MAX_SUPPORTED_RESIDENTIAL_KEYS
            .checked_sub(self.count_credentials()?)
            .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)
    }

    /// Iterates through the credentials.
    ///
    /// If an error is encountered during iteration, it is written to `result`.
    pub fn iter_credentials<'a>(
        &'a self,
        result: &'a mut Result<(), Ctap2StatusCode>,
    ) -> Result<IterCredentials<'a>, Ctap2StatusCode> {
        IterCredentials::new(&self.store, result)
    }

    /// Returns the next creation order.
    pub fn new_creation_order(&self) -> Result<u64, Ctap2StatusCode> {