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

Code before:
        challenge: [u8; 32],
        application: [u8; 32],
        ctap_state: &mut CtapState<R, CheckUserPresence>,
    ) -> Result<Vec<u8>, Ctap1StatusCode>
    where
        R: Rng256,
        CheckUserPresence: Fn(ChannelID) -> Result<(), Ctap2StatusCode>,
    {
        let sk = crypto::ecdsa::SecKey::gensk(ctap_state.rng);
        let pk = sk.genpk();
        let key_handle = ctap_state
            .encrypt_key_handle(sk, &application, None)
            .map_err(|_| Ctap1StatusCode::SW_VENDOR_KEY_HANDLE_TOO_LONG)?;
        if key_handle.len() > 0xFF {
            // This is just being defensive with unreachable code.
            return Err(Ctap1StatusCode::SW_VENDOR_KEY_HANDLE_TOO_LONG);
        }

        let certificate = match ctap_state.persistent_store.attestation_certificate() {
            Ok(Some(value)) => value,
            _ => return Err(Ctap1StatusCode::SW_INS_NOT_SUPPORTED),
        };
        let private_key = match ctap_state.persistent_store.attestation_private_key() {
            Ok(Some(value)) => value,
            _ => return Err(Ctap1StatusCode::SW_INS_NOT_SUPPORTED),
        };

        let mut response = Vec::with_capacity(105 + key_handle.len() + certificate.len());
        response.push(Ctap1Command::LEGACY_BYTE);
        let user_pk = pk.to_uncompressed();
        response.extend_from_slice(&user_pk);
        response.push(key_handle.len() as u8);
        response.extend(key_handle.clone());
        response.extend_from_slice(&certificate);

        // The first byte is reserved.
        let mut signature_data = Vec::with_capacity(66 + key_handle.len());
        signature_data.push(0x00);
        signature_data.extend(&application);
        signature_data.extend(&challenge);
        signature_data.extend(key_handle);
        signature_data.extend_from_slice(&user_pk);

        let attestation_key = crypto::ecdsa::SecKey::from_bytes(private_key).unwrap();
        let signature = attestation_key.sign_rfc6979::<crypto::sha256::Sha256>(&signature_data);

        response.extend(signature.to_asn1_der());
        Ok(response)
    }

    // U2F raw message format specification (version 20170411) section 5.4

Review comment (suggestion):
The `Err` and `None` are technically 2 different errors. The first is a storage error and we might want to use something vendor specific for that, to later translate it to `CTAP2_ERR_VENDOR_INTERNAL_ERROR`.

So how about
> let certificate = ctap_state.persistent_store.attestation_certificate()
 .map_err(Ctap1StatusCode::SW_VENDOR_INTERNAL_ERROR)?
.ok_or(Ctap1StatusCode::SW_INS_NOT_SUPPORTED)?;

Code after:
    // +------+-------------------+-----------------+------------+--------------------+
    // + 0x00 | application (32B) | challenge (32B) | key handle | User pub key (65B) |
    // +------+-------------------+-----------------+------------+--------------------+
    fn process_register<R, CheckUserPresence>(
        challenge: [u8; 32],
        application: [u8; 32],
        ctap_state: &mut CtapState<R, CheckUserPresence>,
    ) -> Result<Vec<u8>, Ctap1StatusCode>
    where
        R: Rng256,
        CheckUserPresence: Fn(ChannelID) -> Result<(), Ctap2StatusCode>,
    {
        let sk = crypto::ecdsa::SecKey::gensk(ctap_state.rng);
        let pk = sk.genpk();
        let key_handle = ctap_state
            .encrypt_key_handle(sk, &application, None)
            .map_err(|_| Ctap1StatusCode::SW_COMMAND_ABORTED)?;
        if key_handle.len() > 0xFF {
            // This is just being defensive with unreachable code.
            return Err(Ctap1StatusCode::SW_VENDOR_KEY_HANDLE_TOO_LONG);
        }

        let certificate = ctap_state
            .persistent_store
            .attestation_certificate()
            .map_err(|_| Ctap1StatusCode::SW_MEMERR)?
            .ok_or(Ctap1StatusCode::SW_COMMAND_ABORTED)?;
        let private_key = ctap_state
            .persistent_store
            .attestation_private_key()
            .map_err(|_| Ctap1StatusCode::SW_MEMERR)?
            .ok_or(Ctap1StatusCode::SW_COMMAND_ABORTED)?;

        let mut response = Vec::with_capacity(105 + key_handle.len() + certificate.len());
        response.push(Ctap1Command::LEGACY_BYTE);
        let user_pk = pk.to_uncompressed();
        response.extend_from_slice(&user_pk);
        response.push(key_handle.len() as u8);
        response.extend(key_handle.clone());
        response.extend_from_slice(&certificate);

        // The first byte is reserved.
        let mut signature_data = Vec::with_capacity(66 + key_handle.len());
        signature_data.push(0x00);
        signature_data.extend(&application);
        signature_data.extend(&challenge);
        signature_data.extend(key_handle);
        signature_data.extend_from_slice(&user_pk);

        let attestation_key = crypto::ecdsa::SecKey::from_bytes(private_key).unwrap();
        let signature = attestation_key.sign_rfc6979::<crypto::sha256::Sha256>(&signature_data);