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

Code before:
    let mut block_iter = ciphertext.chunks_exact(16);
    let iv = if has_iv {
        block_len -= 1;
        let iv_block = block_iter
            .next()
            .ok_or(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)?;
        *array_ref!(iv_block, 0, 16)
    } else {
        [0u8; 16]
    };
    let mut blocks = Vec::with_capacity(block_len);
    for block in block_iter {
        blocks.push(*array_ref!(block, 0, 16));
    }
    let aes_dec_key = crypto::aes256::DecryptionKey::new(aes_enc_key);
    cbc_decrypt(&aes_dec_key, iv, &mut blocks);
    Ok(blocks.iter().flatten().cloned().collect::<Vec<u8>>())
}

#[cfg(test)]
mod test {
    use super::*;
    use crypto::rng256::ThreadRng256;

    #[test]
    fn test_encrypt_decrypt_with_iv() {
        let mut rng = ThreadRng256 {};
        let aes_enc_key = crypto::aes256::EncryptionKey::new(&[0xC2; 32]);
        let plaintext = vec![0xAA; 64];
        let ciphertext = aes256_cbc_encrypt(&mut rng, &aes_enc_key, &plaintext, true).unwrap();
        let decrypted = aes256_cbc_decrypt(&aes_enc_key, &ciphertext, true).unwrap();
        assert_eq!(decrypted, plaintext);
    }

    #[test]
    fn test_encrypt_decrypt_without_iv() {
        let mut rng = ThreadRng256 {};
        let aes_enc_key = crypto::aes256::EncryptionKey::new(&[0xC2; 32]);
        let plaintext = vec![0xAA; 64];
        let ciphertext = aes256_cbc_encrypt(&mut rng, &aes_enc_key, &plaintext, false).unwrap();
        let decrypted = aes256_cbc_decrypt(&aes_enc_key, &ciphertext, false).unwrap();
        assert_eq!(decrypted, plaintext);
    }
}


Review comment (suggestion):
Here we're probably lacking a test to ensure that CBC encryption is actually CBC and using the IV correctly.
The 2 tests here are only testing that with the same parameters, `f(f^-1(x))=x`

Code after:
    let mut block_iter = ciphertext.chunks_exact(16);
    let iv = if embeds_iv {
        block_len -= 1;
        let iv_block = block_iter
            .next()
            .ok_or(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)?;
        *array_ref!(iv_block, 0, 16)
    } else {
        [0u8; 16]
    };
    let mut blocks = Vec::with_capacity(block_len);
    for block in block_iter {
        blocks.push(*array_ref!(block, 0, 16));
    }
    let aes_dec_key = crypto::aes256::DecryptionKey::new(aes_enc_key);
    cbc_decrypt(&aes_dec_key, iv, &mut blocks);
    Ok(blocks.iter().flatten().cloned().collect::<Vec<u8>>())
}

#[cfg(test)]
mod test {
    use super::*;
    use crypto::rng256::ThreadRng256;

    #[test]
    fn test_encrypt_decrypt_with_iv() {
        let mut rng = ThreadRng256 {};
        let aes_enc_key = crypto::aes256::EncryptionKey::new(&[0xC2; 32]);
        let plaintext = vec![0xAA; 64];
        let ciphertext = aes256_cbc_encrypt(&mut rng, &aes_enc_key, &plaintext, true).unwrap();
        let decrypted = aes256_cbc_decrypt(&aes_enc_key, &ciphertext, true).unwrap();
        assert_eq!(decrypted, plaintext);
    }

    #[test]
    fn test_encrypt_decrypt_without_iv() {
        let mut rng = ThreadRng256 {};
        let aes_enc_key = crypto::aes256::EncryptionKey::new(&[0xC2; 32]);
        let plaintext = vec![0xAA; 64];
        let ciphertext = aes256_cbc_encrypt(&mut rng, &aes_enc_key, &plaintext, false).unwrap();
        let decrypted = aes256_cbc_decrypt(&aes_enc_key, &ciphertext, false).unwrap();
        assert_eq!(decrypted, plaintext);
    }

    #[test]
    fn test_correct_iv_usage() {
        let mut rng = ThreadRng256 {};
        let aes_enc_key = crypto::aes256::EncryptionKey::new(&[0xC2; 32]);
        let plaintext = vec![0xAA; 64];
        let mut ciphertext_no_iv =
            aes256_cbc_encrypt(&mut rng, &aes_enc_key, &plaintext, false).unwrap();