RustCrypto / traits

Collection of cryptography-related traits
592 stars 193 forks source link

aead: incremental encryption API #1364

Open tarcieri opened 1 year ago

tarcieri commented 1 year ago

Though an incremental/"streaming" decryption API is unsafe as by design it must disclose unauthenticated plaintext candidates (which can allow an attacker to perpetrate CCAs), incremental encryption is both possible and safe.

We've had a few requests for incremental/"streaming" APIs:

It seems like such an API would need to make ordering presumptions about how AAD and the input message are received, e.g. the first phase would stream AAD input, and the second phase would stream the input message. Such a construction should cover the most common AEAD modes, but wouldn't work for modes that put AAD at the end for whatever reason (can't think of any offhand).

I don't think it makes sense to support incremental/streaming AAD independently of also streaming the message (as originally requested in https://github.com/RustCrypto/traits/issues/62): if one is streamed, both should be streamed, though perhaps there could be a simplified way to avoid AAD if it isn't desired.

I would also suggest using the word "incremental" over "streaming" so as to avoid confusion with the STREAM construction implemented in aead::stream.

newpavlov commented 1 year ago

For completeness sake, I think it's fine to provide "hazmat" streaming decryption API.

Streaming/incremental AEAD encryption/decryption is relatively low-level, so I think we can implement it in a "detached" fashion. Something like:

trait StreamingAeadEnc {
    fn encrypt_chunk(&mut self, buf: InOutBuf<u8>);
    fn finalize(self) -> Tag<Self>;
}

trait StreamingAeadDec {
    fn decrypt_chunk(&mut self, buf: InOutBuf<u8>);
    fn finalize(self, tag: &Tag<Self>) -> Result<()>;
}

Also, I think it's reasonable to limit the streaming API to AEAD modes based on stream ciphers.

tarcieri commented 1 year ago

@newpavlov that's fine for the message, but we also need to handle AAD.

I would suggest trying to get an API for incremental encryption in place first, then circle back on a "hazmat" API for incremental decryption after that has been implemented.

Also per my suggestion in OP, I think the names should be Incremental* or Chunked* to avoid confusion with aead::stream

newpavlov commented 1 year ago

we also need to handle AAD

Yes, we can do it roughly like this:

trait ChunkedAeadEnc {
    type ChunkedAuthEnc: ChunkedAuthEnc;
    fn process_aad_chunk(&mut self, aad: &[u8]);
    fn finalize_aad(self) -> Self::ChunkedAuthEnc;
}

trait ChunkedAeadDec {
    type ChunkedAuthDec: ChunkedAuthDec;
    fn process_aad_chunk(&mut self, aad: &[u8]);
    fn finalize_aad(self) -> Self::ChunkedAuthDec;
}

trait ChunkedAuthEnc {
    fn encrypt_chunk(&mut self, buf: InOutBuf<u8>);
    fn finalize(self) -> Tag<Self>;
}

trait ChunkedAuthDec {
    fn decrypt_chunk(&mut self, buf: InOutBuf<u8>);
    fn finalize(self, tag: &Tag<Self>) -> Result<()>;
}
tarcieri commented 1 year ago

That seems okay aside from the traits to actually initialize the stateful objects for handling AAD.

You can't just implement e.g. ChunkedAeadEnc on the existing AEAD types since they don't hold any state other than the key (expansion).

It might also make sense to have a single API for inputting chunked AAD which can then be branched into encryption or decryption (although if we did that, I think all of the "chunked" APIs should be placed under hazmat to avoid confusion).

tarcieri commented 1 year ago

Separately, I worry a little bit about Chunked* naming as in core/std that terminology is used pretty much exclusively to refer to fixed-sized chunks, whereas these APIs can operate on variable-sized chunks, which is why I suggested "incremental" instead as it's nicely unambiguous

newpavlov commented 1 year ago

That seems okay aside from the traits to actually initialize the stateful objects for handling AAD.

Initialization can be handled by the existing KeyIvInit/InnerIvInit traits.

You can't just implement e.g. ChunkedAeadEnc on the existing AEAD types since they don't hold any state other than the key (expansion).

Yes. The suggested traits should be implemented for newly introduced types. To reduce code duplication we may use them to implement the stateless API, assuming the compiler will be able to properly optimize such code.

which is why I suggested "incremental" instead as it's nicely unambiguous

I am fine with either.

tarcieri commented 1 year ago

There are a couple problems with using KeyIvInit, namely it seems rather counterintuitive (AEAD uses "nonce" rather than "IV" naming, nor does it communicate its purpose as incremental/streaming encryption), and it can't leverage the information from AeadCore::NonceSize.

I think a trait specific to this purpose which bounds on AeadCore and can use AeadCore::NonceSize and "nonce" as the terminology, as well as signaling that the resulting object provides incremental/streaming encryption, would be a lot more clear.

newpavlov commented 1 year ago

AEAD uses "nonce" rather than "IV" naming, nor does it communicate its purpose as incremental/streaming encryption

KeyIvInit is used for both IVs and nonces. It's stated explicitly in its docs. It may be worth to rename it to KeyNonceInit, but it's a separate discussion.

it can't leverage the information from AeadCore::NonceSize

Arguably, AeadCore should use IvSizeUser to define this associated type/constant. IIRC IvSizeUser was introduced later than AeadCore, so we can fix it in the next breaking aead release.

tarcieri commented 1 year ago

I think using anything with "IV" in the name is going to confuse AEAD users where "nonce" is the term-of-art, and documenting that "IV is actually the same as nonce" won't help eliminate that confusion

newpavlov commented 1 year ago

I agree, but the same argument can be applied to stream ciphers and we use the KeIvInit trait to initialize them. Assuming we will rename it to KeyNonceInit, are you fine with using it for AEAD crates?

tarcieri commented 1 year ago

I don't see a problem with stream ciphers, where "IV" is commonly used.

Nobody uses "IV" in the context of AEADs. It's always "nonce".

newpavlov commented 1 year ago

I believe that with dedicated stream ciphers like ChaCha "nonce" is used much more often than "IV". The latter is usually used only in the context of the CTR mode, since it's a traditionally terminology for block modes.

So what do you think about KeyNonceInit? While it will be a bit confusing to use "nonce" in the context of block modes, I think it's a fine tradeoff and it's a good practice to not reuse IVs either way.

tarcieri commented 1 year ago

I would say djb's use of "nonce" with Salsa/ChaCha is something of a neologism.

I've always seen "initialization vector" with older stream ciphers like RC4.

I've never seen "initialization vector" used with AEADs.

tarcieri commented 1 year ago

For the trait to actually be impl'd on the existing AEAD types, I'd think I'd like something shaped roughly like:


pub trait AeadIncremental: AeadCore {
    type Aad: ...;
    type Encryptor: ...;
    type Decryptor: ...;

    fn incremental_with_aad(&self) -> Self::Aad;

    fn incremental_encrypt(&self, aad: &[u8]) -> Self::Encryptor {
        self.incremental_with_aad().update_aad(aad).encrypt()
    }

    fn incremental_decrypt(&self, aad: &[u8]) -> Self::Encryptor {
        self.incremental_with_aad().update_aad(aad).decrypt()
    }
}
tarcieri commented 1 year ago

I'm also not entirely opposed to "chunked" naming. It's easier to type. I just worry about confusion over fixed/variable size.

MasterAwesome commented 1 year ago

Chunked seems misleading because of the fixed size intuition from stdlib. Incremental or Streamed sounds byte granular.

As for the construction AAD seems to be streamed almost always before the plaintexts although, I'm guessing we want to make it such that the trait implementors can change this up tomorrow. How can we support this? If we use the type state to allow AAD only in the beginning we would restrict this ability.

As for streaming aead, I too agree of implementing streaming decryption as well in hazmat to avoid people making worse mistakes implementing it themselves.