Please consider the goats

[qm3ster]

I started this PR to do some absolutely unnecessary nitpicking, but it turned out the crate actually eats data like there is no tomorrow leads to silent corruption in a number of places, by not handling length read and written.

Please update the article at https://kerkour.com/rust-file-encryption-chacha20poly1305-argon2 accordingly, if you have time. (Think of the children!)

The commits are well-isolated, and their commit messages tell a compelling (albeit overly-dramatic) story (or so some say), be sure to read.

Best regards!

[sylvain101010]

Wow, great PR, thank you very much!

But I can't accept it as is, at least for the following reasons.

• I can't accept changes of the dependencies (https://kerkour.com/rust-crate-backdoor)
• Using a BufReader completely defeat the purpose of using our own buffer, because a BufReader maintains it's own internal buffer, which size is out of our control.
• I prefer not to use anyhow::bail! because I find it too obfuscated for my taste

Happy to merge once it's fixed :)

[qm3ster]

Oh noes! And there's still no {read,write}_vectored on Windows! https://github.com/rust-lang/rust/blob/master/library/std/src/sys/windows/handle.rs#L102-L109

Guess I'll have to write something nasty io-wise 🙄. Or we can constrain the OS to actually read only our block size, including the initial even smaller reads of salt and nonce, which is bad perf, but more in line with the complexity of a tutorial article. In that case, it would probably make sense to increase the size of encryption block to at least 8 * 1024, which, considering we are able to write smaller final blocks, would only make the encrypted size smaller, not larger. However, if writing the nasty thing, it would be possible to get rid of the "empty last block" case, which, judging by the fact they use that flag at all, is probably violating an intended mitigation against some potential extension attacks.

Or, what exactly did you mean regarding BufReader?

1. We are in control of the size of its buffer with the ::with_capacity(usize) constructor.
2. Its purpose is reading chunks bigger, and more importantly, unaligned to encryption block, minimizing the number of syscalls. In-usersepace memcopies between our preallocated buffers are absolutely negligible by comparison.
3. However, we cannot get mutable access to the memory location to zeroize it. I do consider that unnecessary though, alongsize zeroizing the nonce and salt. (They're also Copy types, and are probably all over the place. I should have triple-checked the various intos for turning them into GenericArray to make sure I am not making copies if I thought it was sensitive information.

In an unrelated issue, moving the password Zeroizing<String> into the functions (and ultimately into the key derivation function) so it can be zeroized immediately after key derivation and before the {en,de}cryption is probably a good paranoid thing to do. I should add that.