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

Code before:
    fn from(credential: PublicKeyCredentialSource) -> cbor::Value {
        use PublicKeyCredentialSourceField::*;
        let mut private_key = [0; 32];
        credential.private_key.to_bytes(&mut private_key);
        cbor_extend_map_options! {
            credential.unknown_fields,
            CredentialId => Some(credential.credential_id),
            PrivateKey => Some(private_key.to_vec()),
            RpId => Some(credential.rp_id),
            UserHandle => Some(credential.user_handle),
            OtherUi => credential.other_ui,
            CredRandom => credential.cred_random
        }
    }
}

impl PublicKeyCredentialSource {
    pub fn parse_cbor(cbor_value: cbor::Value) -> Option<PublicKeyCredentialSource> {
        use PublicKeyCredentialSourceField::*;
        let mut map = match cbor_value {
            cbor::Value::Map(x) => x,
            _ => return None,
        };

        let credential_id = read_byte_string(&map.remove(&CredentialId.into())?).ok()?;
        let private_key = read_byte_string(&map.remove(&PrivateKey.into())?).ok()?;
        if private_key.len() != 32 {
            return None;
        }
        let private_key = ecdsa::SecKey::from_bytes(array_ref!(private_key, 0, 32))?;
        let rp_id = read_text_string(&map.remove(&RpId.into())?).ok()?;
        let user_handle = read_byte_string(&map.remove(&UserHandle.into())?).ok()?;
        let other_ui = map
            .remove(&OtherUi.into())
            .as_ref()
            .map(read_text_string)
            .transpose()
            .ok()?;
        let cred_random = map
            .remove(&CredRandom.into())
            .as_ref()
            .map(read_byte_string)
            .transpose()
            .ok()?;
        let unknown_fields = map;
        Some(PublicKeyCredentialSource {
            key_type: PublicKeyCredentialType::PublicKey,
            credential_id,
            private_key,
            rp_id,
            user_handle,

Review comment (suggestion):
All the `.ok()` inside of this function are unnecessarily verbose. A single `.ok()` at the call site would be simpler.

Code after:
            CredentialId => Some(credential.credential_id),
            PrivateKey => Some(private_key.to_vec()),
            RpId => Some(credential.rp_id),
            UserHandle => Some(credential.user_handle),
            OtherUi => credential.other_ui,
            CredRandom => credential.cred_random
        }
    }
}

impl TryFrom<cbor::Value> for PublicKeyCredentialSource {
    type Error = Ctap2StatusCode;

    fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
        use PublicKeyCredentialSourceField::*;
        let mut map = extract_map(cbor_value)?;
        let credential_id = extract_byte_string(ok_or_missing(map.remove(&CredentialId.into()))?)?;
        let private_key = extract_byte_string(ok_or_missing(map.remove(&PrivateKey.into()))?)?;
        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(map.remove(&RpId.into()))?)?;
        let user_handle = extract_byte_string(ok_or_missing(map.remove(&UserHandle.into()))?)?;
        let other_ui = map
            .remove(&OtherUi.into())
            .map(extract_text_string)
            .transpose()?;
        let cred_random = map
            .remove(&CredRandom.into())
            .map(extract_byte_string)
            .transpose()?;
        // 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:
        // 1. When a field is deprecated, its tag is reserved and never reused in future versions,
        //    including to be reintroduced with the same semantics. In other words, removing a field
        //    is permanent.
        // 2. OpenSK is never used with a more recent version of the storage. In particular, OpenSK
        //    is never rolled-back.
        // As a consequence, the unknown fields are only reserved fields and don't need to be
        // preserved.
        Ok(PublicKeyCredentialSource {
            key_type: PublicKeyCredentialType::PublicKey,
            credential_id,
            private_key,
            rp_id,
            user_handle,
            other_ui,
            cred_random,