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

Code before:

        let mut auth_data = self.generate_auth_data(&rp_id_hash, flags);
        auth_data.extend(self.persistent_store.aaguid()?);
        // The length is fixed to 0x20 or 0x70 and fits one byte.
        if credential_id.len() > 0xFF {
            return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_RESPONSE_TOO_LONG);
        }
        auth_data.extend(vec![0x00, credential_id.len() as u8]);
        auth_data.extend(&credential_id);
        let cose_key = match pk.to_cose_key() {
            Some(cose_key) => cose_key,
            None => return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_RESPONSE_CANNOT_WRITE_CBOR),
        };
        auth_data.extend(cose_key);
        if use_hmac_extension {
            let extensions = cbor_map! {
                "hmac-secret" => true,
            };
            if !cbor::write(extensions, &mut auth_data) {
                return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_RESPONSE_CANNOT_WRITE_CBOR);
            }
        }

        let mut signature_data = auth_data.clone();
        signature_data.extend(client_data_hash);
        let (signature, x5c) = if USE_BATCH_ATTESTATION {
            let attestation_key =
                crypto::ecdsa::SecKey::from_bytes(self.persistent_store.attestation_private_key()?)
                    .unwrap();
            (
                attestation_key.sign_rfc6979::<crypto::sha256::Sha256>(&signature_data),
                Some(vec![self.persistent_store.attestation_certificate()?]),
            )
        } else {
            (
                sk.sign_rfc6979::<crypto::sha256::Sha256>(&signature_data),
                None,
            )
        };
        let attestation_statement = PackedAttestationStatement {
            alg: SignatureAlgorithm::ES256 as i64,
            sig: signature.to_asn1_der(),
            x5c,
            ecdaa_key_id: None,
        };
        Ok(ResponseData::AuthenticatorMakeCredential(
            AuthenticatorMakeCredentialResponse {
                fmt: String::from("packed"),
                auth_data,
                att_stmt: attestation_statement,
            },

Review comment (performance):
Maybe we can use this opportunity to remove the boolean flag and use the correct supported behavior:
If certificate exists:
  batch attestation
if not:
  self attestation

If do we think that deciding on batch attestation on compile time is a feature, and this way, you can't forget to write a certificate before usage?

Code after:
            let random_id = self.rng.gen_uniform_u8x32().to_vec();
            let credential_source = PublicKeyCredentialSource {
                key_type: PublicKeyCredentialType::PublicKey,
                credential_id: random_id.clone(),
                private_key: sk.clone(),
                rp_id,
                user_handle: user.user_id,
                // This input is user provided, so we crop it to 64 byte for storage.
                // The UTF8 encoding is always preserved, so the string might end up shorter.
                other_ui: user
                    .user_display_name
                    .map(|s| truncate_to_char_boundary(&s, 64).to_string()),
                cred_random,
                cred_protect_policy,
            };
            self.persistent_store.store_credential(credential_source)?;
            random_id
        } else {
            self.encrypt_key_handle(sk.clone(), &rp_id_hash)
        };

        let mut auth_data = self.generate_auth_data(&rp_id_hash, flags);
        auth_data.extend(self.persistent_store.aaguid()?);
        // The length is fixed to 0x20 or 0x70 and fits one byte.
        if credential_id.len() > 0xFF {
            return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_RESPONSE_TOO_LONG);
        }
        auth_data.extend(vec![0x00, credential_id.len() as u8]);
        auth_data.extend(&credential_id);
        let cose_key = match pk.to_cose_key() {
            Some(cose_key) => cose_key,
            None => return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_RESPONSE_CANNOT_WRITE_CBOR),
        };
        auth_data.extend(cose_key);
        if has_extension_output {
            let hmac_secret_output = if use_hmac_extension { Some(true) } else { None };
            let extensions_output = cbor_map_options! {
                "hmac-secret" => hmac_secret_output,
                "credProtect" => cred_protect_policy,
            };
            if !cbor::write(extensions_output, &mut auth_data) {
                return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_RESPONSE_CANNOT_WRITE_CBOR);
            }
        }

        let mut signature_data = auth_data.clone();
        signature_data.extend(client_data_hash);
        // We currently use the presence of the attestation private key in the persistent storage to
        // decide whether batch attestation is needed.
        let (signature, x5c) = match self.persistent_store.attestation_private_key()? {
            Some(attestation_private_key) => {