Fix mismatched key type names

[neekolas]

Impact

Variable names that directly contradict their use in the code may confuse developers leading to implementation errors or could affect the perceived security posture of the application. Such variables may also be evidence of incorrect assumptions, incomplete code refactoring, or actual bugs within the code.

Description

A message history file is encrypted using AES-GCM, as implemented in encrypt_history_file():

fn encrypt_history_file(
    input_path: &Path,
    output_path: &Path,
    encryption_key: &[u8; ENC_KEY_SIZE],
) -> Result<(), MessageHistoryError> {
    // Read the messages file content
    let mut input_file = File::open(input_path)?;
    let mut buffer = Vec::new();
    input_file.read_to_end(&mut buffer)?;

    let nonce = generate_nonce();

    // Create a cipher instance
    let cipher = Aes256Gcm::new(GenericArray::from_slice(encryption_key));
    let nonce_array = GenericArray::from_slice(&nonce);

    // Encrypt the file content
    let ciphertext = cipher.encrypt(nonce_array, buffer.as_ref())?;
}

Figure 14: xmtp_mls/src/groups/message_history.rs

However, when this function is called elsewhere in the code, a ChaCha20Poly1305 key is used:

let enc_key = HistoryKeyType::new_chacha20_poly1305_key();
encrypt_history_file(
    temp_file.as_path(),
    history_file.as_path(),
    enc_key.as_bytes(),
)?;

Figure 15: xmtp_mls/src/groups/message_history.rs

Indeed, the only supported key type for HistoryKeyType is ChaCha20-Poly1305:

#[derive(Copy, Clone, Debug, PartialEq)]
pub(crate) enum HistoryKeyType {
    Chacha20Poly1305([u8; ENC_KEY_SIZE]),
}

Figure 16: xmtp_mls/src/groups/message_history.rs

The approach as implemented works correctly, as the underlying key is just a vector of 32 random bytes. However, the naming conventions are unclear and suggest a partial refactor or change in design has not been fully completed. For clarity, it is recommended to rename the unused ChaCha20-Poly1305 types to match the usage of AES-GCM. Alternatively, if support for both is required, the implementation should be updated with support for both algorithms based on the HistoryKeyType enum.

Similarly, it was observed that encrypt_history_file() expects the key as encryption_key: &[u8; ENC_KEY_SIZE], whereas decrypt_history_file() expects encryption_key: MessageHistoryKeyType. In general, one would expect the types between these two functions to match. This is particularly important if multiple algorithms are supported, as algorithm confusion attacks may apply if different constraints are applied at encryption vs decryption. If support for both encryption algorithms is required, it should not be possible to mistakenly attempt decryption with the incorrect key type.

Recommendation

Review the highlighted code snippets and either:

1.	Rename unused ChaCha20-Poly1305 types to reflect AES-GCM use.
2.	Add support for both algorithms if needed by aligning with the HistoryKeyType enum.
3.	Ensure that encryption and decryption functions use consistent key types.

Location

xmtp_mls/src/groups/message_history.rs

[codabrink]

This has not been fixed yet. I'll make the changes necessary in the consent sync PR.