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

Code before:
    }

    /// Returns the PIN hash if defined.
    pub fn pin_hash(&self) -> Result<Option<[u8; PIN_AUTH_LENGTH]>, Ctap2StatusCode> {
        Ok(self.pin_properties()?.map(|p| p.hash))
    }

    /// Returns the length of the currently set PIN if defined.
    pub fn pin_code_point_length(&self) -> Result<Option<u8>, Ctap2StatusCode> {
        Ok(self.pin_properties()?.map(|p| p.code_point_length))
    }

    /// Sets the PIN hash and length.
    ///
    /// If it was already defined, it is updated.
    pub fn set_pin(
        &mut self,
        pin_hash: &[u8; PIN_AUTH_LENGTH],
        pin_code_point_length: u8,
    ) -> Result<(), Ctap2StatusCode> {
        let mut pin_properties = [0; 1 + PIN_AUTH_LENGTH];
        pin_properties[0] = pin_code_point_length;
        pin_properties[1..].clone_from_slice(pin_hash);
        self.store.insert(key::PIN_PROPERTIES, &pin_properties)?;
        // If this second transaction fails, you are forced to retry.
        Ok(self.store.remove(key::FORCE_PIN_CHANGE)?)
    }

    /// Returns the number of remaining PIN retries.
    pub fn pin_retries(&self) -> Result<u8, Ctap2StatusCode> {
        match self.store.find(key::PIN_RETRIES)? {
            None => Ok(MAX_PIN_RETRIES),
            Some(value) if value.len() == 1 => Ok(value[0]),
            _ => Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR),
        }
    }

    /// Decrements the number of remaining PIN retries.
    pub fn decr_pin_retries(&mut self) -> Result<(), Ctap2StatusCode> {
        let old_value = self.pin_retries()?;
        let new_value = old_value.saturating_sub(1);
        if new_value != old_value {
            self.store.insert(key::PIN_RETRIES, &[new_value])?;
        }
        Ok(())
    }

    /// Resets the number of remaining PIN retries.
    pub fn reset_pin_retries(&mut self) -> Result<(), Ctap2StatusCode> {
        Ok(self.store.remove(key::PIN_RETRIES)?)
    }

Review comment (bug):
Just  checking the semantics we want in case of power-loss. Is it ok to only insert and not remove? Is there a reason to not use a transaction? There are at least 2 reasons not to:
1. It costs 2 words of lifetime.
2. Those 2 words of lifetime are also temporary required capacity, so if the store is almost full, the transaction would fail, while both updates in a sequence would pass. But this scenario should not be considered since we should probably never hit capacity because of how we dimensionned the CTAP storage.

Not sure there are many reasons to use a transaction, but we could still add a comment to say that it's ok to do in 2 steps (because the relation between both entries is only in one direction and not an equivalence).

Code after:
        }
    }

    /// Returns the PIN hash if defined.
    pub fn pin_hash(&self) -> Result<Option<[u8; PIN_AUTH_LENGTH]>, Ctap2StatusCode> {
        Ok(self.pin_properties()?.map(|p| p.hash))
    }

    /// Returns the length of the currently set PIN if defined.
    pub fn pin_code_point_length(&self) -> Result<Option<u8>, Ctap2StatusCode> {
        Ok(self.pin_properties()?.map(|p| p.code_point_length))
    }

    /// Sets the PIN hash and length.
    ///
    /// If it was already defined, it is updated.
    pub fn set_pin(
        &mut self,
        pin_hash: &[u8; PIN_AUTH_LENGTH],
        pin_code_point_length: u8,
    ) -> Result<(), Ctap2StatusCode> {
        let mut pin_properties = [0; 1 + PIN_AUTH_LENGTH];
        pin_properties[0] = pin_code_point_length;
        pin_properties[1..].clone_from_slice(pin_hash);
        Ok(self.store.transaction(&[
            StoreUpdate::Insert {
                key: key::PIN_PROPERTIES,
                value: &pin_properties[..],
            },
            StoreUpdate::Remove {
                key: key::FORCE_PIN_CHANGE,
            },
        ])?)
    }

    /// Returns the number of remaining PIN retries.
    pub fn pin_retries(&self) -> Result<u8, Ctap2StatusCode> {
        match self.store.find(key::PIN_RETRIES)? {
            None => Ok(MAX_PIN_RETRIES),
            Some(value) if value.len() == 1 => Ok(value[0]),
            _ => Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR),
        }
    }

    /// Decrements the number of remaining PIN retries.
    pub fn decr_pin_retries(&mut self) -> Result<(), Ctap2StatusCode> {
        let old_value = self.pin_retries()?;
        let new_value = old_value.saturating_sub(1);
        if new_value != old_value {
            self.store.insert(key::PIN_RETRIES, &[new_value])?;
        }