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

Code before:
    }
    let pad_length = MAX_PADDING_LENGTH - (data.len() as u8 - 1);
    data.extend(core::iter::repeat(pad_length).take(pad_length as usize));
    Ok(())
}

fn remove_padding(data: &mut Vec<u8>) -> Result<(), Ctap2StatusCode> {
    if data.len() != MAX_PADDING_LENGTH as usize + 1 {
        // This is an internal error instead of corrupted credential ID which we should just ignore because
        // we've already checked that the HMAC matched.
        return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR);
    }
    let pad_length = *data.last().unwrap();
    if pad_length == 0 || pad_length > MAX_PADDING_LENGTH {
        return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR);
    }
    if !data
        .drain((data.len() - pad_length as usize)..)
        .all(|x| x == pad_length)
    {
        return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR);
    }
    Ok(())
}

/// Encrypts the given private key, relying party ID hash, and cred protect policy into a credential ID.
///
/// Other information, such as a user name, are not stored. Since encrypted credential IDs are
/// stored server-side, this information is already available (unencrypted).
pub fn encrypt_to_credential_id(
    env: &mut impl Env,
    private_key: &PrivateKey,
    rp_id_hash: &[u8; 32],
    cred_protect_policy: Option<CredentialProtectionPolicy>,
    cred_blob: Option<Vec<u8>>,
) -> Result<Vec<u8>, Ctap2StatusCode> {
    let mut payload = Vec::new();
    let cbor = cbor_map_options! {
      CredentialSourceField::PrivateKey => private_key,
      CredentialSourceField::RpIdHash=> rp_id_hash,
      CredentialSourceField::CredProtectPolicy => cred_protect_policy,
      CredentialSourceField::CredBlob => cred_blob,
    };
    cbor_write(cbor, &mut payload)?;
    add_padding(&mut payload)?;

    let aes_enc_key = crypto::aes256::EncryptionKey::new(&env.key_store().key_handle_encryption()?);
    let encrypted_payload = aes256_cbc_encrypt(env.rng(), &aes_enc_key, &payload, true)?;
    let mut credential_id = encrypted_payload;
    credential_id.insert(0, CBOR_CREDENTIAL_ID_VERSION);


Review comment (security):
The description needs to be updated:
`/// Encrypts the given private key, relying party ID hash, and cred protect policy into a credential ID.`

Maybe a more general phrasing is easier to maintain.

Code after:
fn remove_padding(data: &mut Vec<u8>) -> Result<(), Ctap2StatusCode> {
    if data.len() != MAX_PADDING_LENGTH as usize + 1 {
        // This is an internal error instead of corrupted credential ID which we should just ignore because
        // we've already checked that the HMAC matched.
        return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR);
    }
    let pad_length = *data.last().unwrap();
    if pad_length == 0 || pad_length > MAX_PADDING_LENGTH {
        return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR);
    }
    if !data
        .drain((data.len() - pad_length as usize)..)
        .all(|x| x == pad_length)
    {
        return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR);
    }
    Ok(())
}

/// Encrypts the given private key, relying party ID hash, and some other metadata into a credential ID.
///
/// Other information, such as a user name, are not stored. Since encrypted credential IDs are
/// stored server-side, this information is already available (unencrypted).
pub fn encrypt_to_credential_id(
    env: &mut impl Env,
    private_key: &PrivateKey,
    rp_id_hash: &[u8; 32],
    cred_protect_policy: Option<CredentialProtectionPolicy>,
    cred_blob: Option<Vec<u8>>,
) -> Result<Vec<u8>, Ctap2StatusCode> {
    let mut payload = Vec::new();
    let cbor = cbor_map_options! {
      CredentialSourceField::PrivateKey => private_key,
      CredentialSourceField::RpIdHash=> rp_id_hash,
      CredentialSourceField::CredProtectPolicy => cred_protect_policy,
      CredentialSourceField::CredBlob => cred_blob,
    };
    cbor_write(cbor, &mut payload)?;
    add_padding(&mut payload)?;

    let aes_enc_key = crypto::aes256::EncryptionKey::new(&env.key_store().key_handle_encryption()?);
    let encrypted_payload = aes256_cbc_encrypt(env.rng(), &aes_enc_key, &payload, true)?;
    let mut credential_id = encrypted_payload;
    credential_id.insert(0, CBOR_CREDENTIAL_ID_VERSION);

    let id_hmac = hmac_256::<Sha256>(
        &env.key_store().key_handle_authentication()?,
        &credential_id[..],
    );
    credential_id.extend(&id_hmac);
    Ok(credential_id)