Skip to content
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: incremental encryption API #1364

Open
tarcieri opened this issue Oct 23, 2023 · 16 comments
Open

aead: incremental encryption API #1364

tarcieri opened this issue Oct 23, 2023 · 16 comments
Labels
aead Authenticated Encryption with Associated Data (AEAD) crate

Comments

@tarcieri
Copy link
Member

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 #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
Copy link
Member

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
Copy link
Member Author

tarcieri commented Oct 23, 2023

@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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member Author

tarcieri commented Oct 23, 2023

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
Copy link
Member

newpavlov commented Oct 23, 2023

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
Copy link
Member Author

tarcieri commented Oct 23, 2023

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
Copy link
Member

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
Copy link
Member Author

tarcieri commented Oct 23, 2023

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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

newpavlov commented Oct 23, 2023

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

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

@MasterAwesome
Copy link

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.

@newpavlov newpavlov added the aead Authenticated Encryption with Associated Data (AEAD) crate label Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aead Authenticated Encryption with Associated Data (AEAD) crate
Projects
None yet
Development

No branches or pull requests

4 participants
@tarcieri @newpavlov @MasterAwesome and others