neosmart / cryptostream

Read and Write stream adapters for on-the-fly encryption and decryption for rust
https://neosmart.net/blog/2018/transparent-encryption-and-decryption-in-rust-with-cryptostreams/
Other
23 stars 6 forks source link

Incorrect decryption when variable read length from source #7

Closed blmarket closed 4 years ago

blmarket commented 4 years ago

Hi,

I think this library have some problem as it silently generates incorrect decryption result to the buffer. Even though I'm moving away from using this library due to the lack of my own time to fix, I created some unit tests to reproduce the issue.

My application to decrypt network stream resulted incorrect output, which was not happened when I buffer everything in memory. Following test case shows decryption result gets incorrect if source Read yields smaller chunks.

use std::io;
use std::io::{BufReader, Read};

use cryptostream::bufread;
use hex;
use openssl::symm::{Cipher, Crypter, Mode};

struct RandomRead<R>
where
    R: Read,
{
    inner: R,
    rng: rand::rngs::ThreadRng,
}

impl<R: Read> RandomRead<R> {
    pub fn new(inner: R) -> Self {
        RandomRead {
            inner,
            rng: rand::thread_rng(),
        }
    }
}

impl<R: Read> Read for RandomRead<R> {
    fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
        use rand::prelude::*;

        let bufsize: usize = if buf.len() <= 5 {
            buf.len()
        } else {
            std::cmp::max((self.rng.gen::<u32>() % buf.len() as u32) as usize, 5)
        };

        let result = self.inner.read(&mut buf[..bufsize])?;
        Ok(result)
    }
}

fn main() -> std::io::Result<()> {
    use openssl::rand::rand_bytes;

    let mut plaintext = vec![0u8; 32 * 1024 * 1024 + 5];
    rand_bytes(&mut plaintext)?;
    let key = b"\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0A\x0B\x0C\x0D\x0E\x0F";
    let iv = b"\x00\x01\x02\x03\x04\x05\x06\x07\x00\x01\x02\x03\x04\x05\x06\x07";

    println!("{}", hex::encode(&plaintext[..10]));

    let mut encrypted = vec![0u8; 32 * 1024 * 1024 + 128];

    let cipher = Cipher::aes_128_cbc();
    let mut crypter = Crypter::new(cipher, Mode::Encrypt, key, Some(iv))?;

    let resultsize = crypter.update(&plaintext, &mut encrypted)?;
    let resultsize2 = crypter.finalize(&mut encrypted[resultsize..])?;
    encrypted.truncate(resultsize + resultsize2);

    {
        let mut decrypted = vec![0u8; 32 * 1024 * 1024 + 128];
        let mut decrypter = Crypter::new(cipher, Mode::Decrypt, key, Some(iv))?;
        let r2 = decrypter.update(&encrypted, &mut decrypted)?;
        let r3 = decrypter.finalize(&mut decrypted[r2..])?;
        decrypted.truncate(r2 + r3);

        assert_eq!(plaintext.len(), decrypted.len());
        println!("{}", plaintext == decrypted); // true, as expected
    }

    {
        let bufreader = BufReader::new(&encrypted[..]);
        let mut decryptor = bufread::Decryptor::new(bufreader, cipher, key, iv)?;
        let mut test: Vec<u8> = Vec::new();
        decryptor.read_to_end(&mut test)?;

        assert_eq!(plaintext.len(), test.len());
        println!("{}", plaintext == test); // true, it works fine if source have good chunks
    }

    {
        let bufreader = BufReader::new(RandomRead::new(&encrypted[..]));
        let mut decryptor = bufread::Decryptor::new(bufreader, cipher, key, iv)?;
        let mut test: Vec<u8> = Vec::new();
        decryptor.read_to_end(&mut test)?;

        assert_eq!(plaintext.len(), test.len());
        println!("{}", plaintext == test); // should be true, but...
    }

    Ok(())
}

My 2 cents: Read in Cryptostream does not check read size properly after reading from encrypted source.

mqudsi commented 4 years ago

Hello,

Is this with the latest release or from git?

mqudsi commented 4 years ago

OK, I was able to find the issue. It was actually pretty convoluted and relied on being supplied with a buffer that was just the right size (it had to cause the first read to result in an incomplete block being fed to the crypter but the second read to be small enough to be written directly to the output without using the overflow buffer).

Thank you for reporting it and for the good test case!

mqudsi commented 4 years ago

I've just pushed out v0.3.1 https://github.com/neosmart/cryptostream/releases/tag/0.3.1 and published the crate.

A version of your test code has been included in the tests, with some brief comments linking back here. @blmarket I want to thank you for taking the time to report the issue and share your test code. It's much appreciated!