-
Notifications
You must be signed in to change notification settings - Fork 198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
aead::stream API is very awkward #1305
Comments
You've more or less reinvented https://github.com/RustCrypto/nacl-compat/tree/master/crypto_secretstream There are some pretty big drawbacks to that design though which make STREAM much more flexible: when used with fixed-sized segments, STREAM permits random access with zero framing overhead. This is not possible with a As another point of feedback: the reason I guess we've discussed patterns for initializing STREAM quite a bit on Zulip but don't have a tracking issue for it. Both the Tink paper and https://eprint.iacr.org/2020/1019.pdf discuss methods of deriving a unique key per stream, which would help address this problem and allow for the use of a random nonce. |
I opened #1306 to track improving STREAM initialization. Otherwise, I would suggest using |
Indeed this didn't come up in my initial research. Probably because I didn't look under
If the underlying TBH, implementing
It was my understanding that if you use |
As the rustdoc notes, these types implement the 𝒟 decryptor and ℰ encryptor objects as described in the STREAM paper, which are explicitly designed to manage the counter for you. Random access is possible via the
You're adding an empty segment as a simplification. While that works, particularly for the packet-framed case, it's not a zero-cost abstraction: it adds an additional MAC tag, and if you're framing the packets, an additional length prefix. These may be undesirable in e.g. the file decryption case.
It's fine with a cipher with an extended nonce, but not fine for any e.g. IETF AEAD, which is why as #1306 notes it isn't something that should be made into a general-purpose pattern. |
I (in my wrappers) don't actually do that, I detect the end of the stream by the fact that the encrypted stream has no more chunks. In fact that's my point: If you wanted to use |
In the file encryption case, you can either know you're at EOF via the filesystem API, or if you have a footer it can tell you when the encrypted ends |
I'm going to close this issue. Your code example is misusing STREAM. The "last block" flag in the nonce is very much a deliberate design decision directly from the STREAM paper: it's used to prevent truncation attacks, where an attacker can trick you into accepting a STREAM which is shorter than the original. Yes, that makes the API a bit painful, but it's there for a reason. |
No, it controls how the nonce is computed, so it needs to be known in advance prior to decryption. Instead, you need to use EOF to detect it, or failing that some outer framing. It might still be possible to build the sort of abstraction you want on top of STREAM (essentially a buffered |
Here's an updated version that uses Codeuse std::cmp::min;
use std::collections::VecDeque;
use std::io::{ErrorKind, Read, Write};
use anyhow::Result;
use chacha20poly1305::aead::stream::{DecryptorBE32, EncryptorBE32};
use chacha20poly1305::KeyInit;
use chacha20poly1305::XChaCha20Poly1305;
use hkdf::Hkdf;
use rand::rngs::OsRng;
use rand::RngCore;
use sha2::Sha256;
struct XChaCha20Poly1305StreamEncryptor<W> {
sink: W,
chunk_size: u64,
encryptor: Option<EncryptorBE32<XChaCha20Poly1305>>,
chunk_buf: Vec<u8>,
}
impl<W> XChaCha20Poly1305StreamEncryptor<W> {
// The stream module doesn't seem to have a generate_nonce() function to generate the required
// 19-byte nonce
fn generate_nonce() -> [u8; 19] {
let mut nonce = [0u8; 19];
OsRng.fill_bytes(&mut nonce);
nonce
}
}
impl<W: Write> XChaCha20Poly1305StreamEncryptor<W> {
pub fn new(mut sink: W, chunk_size: u64, key: &[u8]) -> Result<Self> {
// Derive an XChaCha20 key
let hk = Hkdf::<Sha256>::new(None, key);
let mut derived_key = [0u8; 32];
hk.expand(&[], &mut derived_key).expect("invalid length");
// Initialize encryption
let cipher = XChaCha20Poly1305::new(&derived_key.into());
let nonce = XChaCha20Poly1305StreamEncryptor::<W>::generate_nonce();
let encryptor = EncryptorBE32::from_aead(cipher, &nonce.into());
// Prepend ciphertext with the chunk size
sink.write(&(chunk_size as u64).to_be_bytes())?;
// Prepend ciphertext with the nonce
sink.write(&nonce)?;
Ok(Self {
sink,
encryptor: Some(encryptor),
chunk_size,
chunk_buf: Vec::with_capacity(chunk_size as usize),
})
}
fn emit_chunk(&mut self, last: bool) -> std::io::Result<()> {
let mut e = self.encryptor.take().unwrap();
let encrypted = if last {
e.encrypt_last(self.chunk_buf.as_slice())
.map_err(|e| std::io::Error::new(ErrorKind::Other, e))?
} else {
let encrypted = e
.encrypt_next(self.chunk_buf.as_slice())
.map_err(|e| std::io::Error::new(ErrorKind::Other, e))?;
self.encryptor.replace(e);
encrypted
};
self.sink.write_all(&encrypted)?;
self.chunk_buf.clear();
Ok(())
}
}
impl<W: Write> Write for XChaCha20Poly1305StreamEncryptor<W> {
fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
// We only process what fits in one chunk because I believe it's recommended to create
// backpressure instead of buffering arbitrary amounts of data
let to_read = min(self.chunk_size as usize - self.chunk_buf.len(), buf.len());
self.chunk_buf.extend(&buf[0..to_read]);
debug_assert_eq!(self.chunk_buf.capacity(), self.chunk_size as usize);
if self.chunk_buf.len() == self.chunk_size as usize {
self.emit_chunk(false)?;
}
Ok(to_read as usize)
}
fn flush(&mut self) -> std::io::Result<()> {
self.emit_chunk(true)
}
}
struct XChaCha20Poly1305StreamDecryptor<R> {
source: R,
chunk_size: u64,
decryptor: Option<DecryptorBE32<XChaCha20Poly1305>>,
decrypted_chunk_buf: VecDeque<u8>,
chunk_stash: Vec<u8>,
}
impl<R: Read> XChaCha20Poly1305StreamDecryptor<R> {
pub fn new(mut source: R, key: &[u8]) -> Result<Self> {
// Read chunk size and nonce back
let mut buf = [0u8; 8];
source.read_exact(&mut buf)?;
let chunk_size = u64::from_be_bytes(buf);
let mut nonce = [0u8; 19];
source.read_exact(&mut nonce)?;
// Derive an XChaCha20 key
let hk = Hkdf::<Sha256>::new(None, key);
let mut derived_key = [0u8; 32];
hk.expand(&[], &mut derived_key).expect("invalid length");
// Initialize decryption
let cipher = XChaCha20Poly1305::new(&derived_key.into());
let decryptor = DecryptorBE32::from_aead(cipher, &nonce.into());
let mut instance = Self {
source,
chunk_size,
decryptor: Some(decryptor),
decrypted_chunk_buf: VecDeque::with_capacity(chunk_size as usize * 2),
chunk_stash: Vec::with_capacity(chunk_size as usize + 16),
};
// Read the first chunk into the stash
instance.chunk_stash = XChaCha20Poly1305StreamDecryptor::read_chunk(&mut instance, chunk_size + 16)?;
Ok(instance)
}
fn read_chunk(&mut self, read_chunk_size: u64) -> Result<Vec<u8>> {
let mut encrypted_chunk = Vec::with_capacity(read_chunk_size as usize);
let _ = (&mut self.source)
.take(read_chunk_size)
.read_to_end(&mut encrypted_chunk)?;
Ok(encrypted_chunk)
}
}
impl<R: Read> Read for XChaCha20Poly1305StreamDecryptor<R> {
fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
let read_chunk_size = self.chunk_size + 16;
// Refill the chunk buffer if it holds less than a full chunk (pretty arbitrary)
if (self.decrypted_chunk_buf.len() as u64) < self.chunk_size {
// Already read the next chunk so we can take a peek
let next_chunk = self
.read_chunk(read_chunk_size)
.map_err(|e| std::io::Error::new(ErrorKind::Other, e))?;
// Decrypt if not EOF
if self.chunk_stash.len() > 0 {
let mut d = self.decryptor.take().unwrap();
let decrypted = if next_chunk.len() == 0 {
// This is the last chunk before EOF
d.decrypt_last(self.chunk_stash.as_slice())
.map_err(|e| std::io::Error::new(ErrorKind::Other, e))?
} else {
let decrypted = d
.decrypt_next(self.chunk_stash.as_slice())
.map_err(|e| std::io::Error::new(ErrorKind::Other, e))?;
self.decryptor.replace(d);
decrypted
};
self.decrypted_chunk_buf.extend(decrypted.iter());
}
self.chunk_stash = next_chunk;
}
let drain_count = min(buf.len(), self.decrypted_chunk_buf.len());
for (i, b) in self.decrypted_chunk_buf.drain(0..drain_count).enumerate() {
buf[i] = b;
}
Ok(drain_count)
}
}
#[cfg(test)]
mod tests {
use std::io::{Cursor, Read, Seek, Write};
use anyhow::Result;
use chacha20poly1305::aead::stream::{DecryptorBE32, EncryptorBE32};
use chacha20poly1305::KeyInit;
use chacha20poly1305::XChaCha20Poly1305;
use hkdf::Hkdf;
use sha2::Sha256;
use crate::cryptostream::{XChaCha20Poly1305StreamDecryptor, XChaCha20Poly1305StreamEncryptor};
#[test]
fn stream_test() -> Result<()> {
let passphrase = "correct horse battery staple";
let cleartext = "Das Pferd frisst keinen Gurkensalat";
let chunk_size = 1000;
let mut ciphertext = Cursor::new(Vec::new());
let mut encryptor = XChaCha20Poly1305StreamEncryptor::new(&mut ciphertext, chunk_size, passphrase.as_bytes())?;
encryptor.write_all(cleartext.as_bytes())?;
encryptor.flush()?;
ciphertext.rewind()?;
let mut ciphertext_decrypted = Vec::new();
let mut decryptor = XChaCha20Poly1305StreamDecryptor::new(ciphertext, passphrase.as_bytes())?;
let _ = decryptor.read_to_end(&mut ciphertext_decrypted)?;
assert_eq!(ciphertext_decrypted, cleartext.as_bytes());
Ok(())
}
} |
I also feel the stream API is really awkward. I have some code using the |
Please see #1306 for alternative stream initialization designs |
I used
aead::stream
and it was quite painful.Read
orWrite
orIterator
. You have to break your data up into chunks and encrypt one chunk at a time.encrypt_last()
, although I'm relatively sure that this isn't actually true - encrypting the last chunk withencrypt_next()
works just fine.decrypt_last()
method, but it's unclear how/when you would use it. The only way to know that you're at the last chunk is if you had encrypted an empty chunk at the end. (Granted, encrypting an empty chunk at the end usually happens by accident anyway because that's the way EOF is signaled if you're reading your cleartext from a file.) You could then detect the end by checking if the remaining chunk size is 16, which indicates an empty chunk. This feels awkward but luckily thedecrypt_last()
method doesn't actually have to be called.encrypt()
anddecrypt()
methods and I'm pretty sure those should never be called.generate_nonce()
function to generate the required 19-byte nonce, I had to find the length by trial-and-error.To help with those issues I wrote an encryptor/decryptor pair that wraps
aead::stream::EncryptorBE32/DecryptorBE32
and implementsWrite
(for encryption) andRead
(for decryption).Here they are:
Code
A couple of things of note:
Result<Self>
. This is not something I see often, so maybe this is considered bad style and the I/O should be delayed until the first read/write.Result
s are currentlyanyhow::Result
s, also not something I see often.Is this something that could have a place as part of RustCrypto? Or does it make more sense to publish it as a crate of my own? Or is it just a bad idea in general? (I might be missing something here.)
The text was updated successfully, but these errors were encountered: