aead: API with separate buffers for input and output

[ia0]

Would it make sense to add the following methods to AeadInPlace (similarly for AeadMutInPlace) with default implementations based on the existing methods?

fn encrypt_mut(&self, nonce: &Nonce, aad: &[u8], plain: &[u8], cipher_tag: &mut [u8]) -> Result<()> { ... }
fn encrypt_mut_detached(&self, nonce: &Nonce, aad: &[u8], plain: &[u8], cipher: &mut [u8]) -> Result<Tag> { ... }

fn decrypt_mut(&self, nonce: &Nonce, aad: &[u8], cipher_tag: &[u8], plain: &mut [u8]) -> Result<()> { ... }
fn decrypt_mut_detached(&self, nonce: &Nonce, aad: &[u8], cipher: &[u8], tag: &Tag, plain: &mut [u8]) -> Result<()> { ... }

This would add support (without extra copies or allocation) for implementations that don't support aliasing input and output. They would implement the _mut version and use a copy to implement the _in_place version using the _mut version.

[tarcieri]

We could potentially add something like this, but I'll note the API surface is already pretty complicated.

I'm not sure it makes sense to add these to AeadInPlace: they're not in-place operations. But we could potentially add yet another trait with a blanket impl for types which impl AeadInPlace.

[ia0]

I agree the current API is complicated but may I suggest to make it more complicated to possibly make it simpler:

• There are 6 "implementer" traits: one per type of implementation (in-place, 2 buffers, or alloc on one side and stateful or stateless on the other, so 6 traits). Those traits would only be used by implementers.
• There are 6 "user" traits: one per type of usage (similar to the implementer traits but with a different name). Those traits would only be used by users.
• A blanket impl from each implementer trait to each user trait when possible. Note that some user traits may not be implementable by any implementer trait.

[tarcieri]

Pretty much all of our AEADs impl AeadCore+AeadInPlace, and there's already a blanket impl of Aead for AeadCore.

The *Mut traits are pretty much there to support hardware accelerators and we don't otherwise impl stateful AEADs. There are already blanket impls linking the non-mut and *Mut traits.

We don't really have a distinction of "implementer" versus "user" traits: each trait is independently useful to end users.

Such a distinction exists in the digest crate e.g. digest::core_api but I'm not sure that would be something we want to carry over here.

[ia0]

Yes I understand the current design, I'm suggesting to fix its limitations, in particular the fact that blanket impls force extra copy when implemented and used interfaces differ. Here's a concrete example:

• Let's call AeadNoAlloc the 2 buffers version.
• Let's assume we have a blanket impl from AeadInPlace to AeadNoAlloc.
• Let's assume the hardware only provides a 2 buffers API.
• You either have to only implement AeadNoAlloc and prevent users from using AeadInPlace. Or worse you have to implement AeadInPlace with allocation and copy, then when the user wants to use AeadNoAlloc they pay this price plus another copy from the blanket impl.

By dissociating implementer and user interfaces you provide as much choice to the user as possible, paying only the minimum interface mismatch cost.

I can see how it may be more complicated by having much more traits, but the design is also easier to understand: if you're a user, just use the trait (possibly multiple) you want from those, if you're an implementer, just implement the trait (and only one) you want from those.

No worries if that's too much complexity. I'm anyway currently adding the missing traits myself. It's just that it could be useful for others too.

[tarcieri]

To back up, what you want isn't an in-place API, but one which has an immutable reference to the source and a mutable reference to the destination buffer. So, it doesn't make sense to put such methods on the AeadInPlace trait, because they aren't in-place methods (which definitionally operate over the same input/output buffer).

So what you're asking for only makes sense as a new trait.

I'm suggesting to fix its limitations, in particular the fact that blanket impls force extra copy when implemented and used interfaces differ.

I don't think this scenario would happen. The blanket impl would exist for types which impl AeadInPlace, but for backends which don't support in-place operation and natively prefer separate source/destination buffers, you wouldn't impl AeadInPlace, only the new traits.

Note that AeadMut already exists, so the encrypt_mut* and decrypt_mut* names don't make sense.

What might make sense is adding methods to the Aead trait, and naming them something like encrypt_into and decrypt_into. That would probably be a breaking change though.

[ia0]

what you want isn't an in-place API

Indeed, the naming is wrong. I called it like that because I also consider in-place the version of an API that takes an output buffer compared to one that allocates and returns a new buffer, because it essentially operates in-place instead of creating a new-place. But let's stick to the current naming which makes more sense when there's both an input and an output buffer.

I don't think this scenario would happen. The blanket impl would exist for types which impl AeadInPlace, but for backends which don't support in-place operation and natively prefer separate source/destination buffers, you wouldn't impl AeadInPlace, only the new traits.

Yes, that's also an option. But then, you put the weight to handle this impedance on the user. Because now users can't just use an in-place API and be portable over all implementations. They need somehow to check whether the implementation supports an in-place API and if not, use another API. This argument is symmetric, I used in-place, but the user should be able to use any API A regardless of whether the implementation implements A, B, or C, since they're all equivalent modulo possible copy and/or allocation.

That would probably be a breaking change though.

Yes, I think any solution will be a breaking change. Only adding a new independent trait would not be a breaking change. That would not be the best user experience, but that would be fine for me. I'll implement my own impedance matching traits on top.

[tarcieri]

Because now users can't just use an in-place API and be portable over all implementations.

...but you want to offer a trait specifically for implementations which don't offer an in-place API and require two separate and unaliased input and output buffers as opposed to being able to operate over the same buffer for input and output.

If we offer such a trait, definitionally it will be for implementations which don't support in-place operation. Them's the brakes.

The Aead trait can still be portable over all implementations, though the buffer management will be a little complicated for the blanket impl for in-place use (it would probably need to just call Aead::encrypt/Aead::decrypt and copy the output into the provided buffer, making for two copies in total)

[newpavlov]

We have the inout crate specifically to generalize over in-place and buffer-to-buffer modes of operation. The cipher crates are already implemented on top of it. Currently inout supports only cases when input and output data is of equal length, so it would be limited to AEADs based on stream ciphers used in detached mode.

UPD: I forgot about InOutBufReserved, it was designed for padded block modes, so it would be a bit difficult to use for AEADs which store tag as prefix.

[tarcieri]

Adding inout support to the detached modes sounds like a good idea to me

[ia0]

Yes, the Aead can be used as the only portable user trait, but then if it happens that the user needs an in-place version there would be unnecessary copies. Maybe there's an argument that only Aead should be used, but that's not clear to me yet.

Just to make sure we understand ourselves correctly, let me try to illustrate the situation as I see it. I'll focus only on encryption and ignore the nonce and aad. I'll also abuse notations to mention the length. We have the following functions:

buffer	tag	arguments	results	name
alloc	attached	&[u8; N]	Vec<u8; N + T>	encrypt
alloc	detached	&[u8; N]	Vec<u8; N>, Tag	does not exist
in-place	attached	&mut dyn Buffer<N, N + T>	-	encrypt_in_place
in-place	detached	&mut [u8; N]	Tag	encrypt_in_place_detached
separate	attached	&[u8; N], &mut [u8; N + T]	-	suggested encrypt_into
separate	detached	&[u8; N], &mut [u8; N]	Tag	suggested encrypt_into_in_place

You cannot create a blanket impl topological order between those 6 equivalent functions. That's maybe where I'm wrong, so please provide such order if it exists. Here is my proof that there are no such order:

• Let's assume there's 2 implementers: in-place detached and separate detached.
• Let's assume there's 2 users: in-place detached and separate detached.
• Let's assume users want to be generic over the implementation.
• Let's assume we don't want unnecessary copies/allocations.
• Let's assume there is a way to order between in-place detached and separate detached and call B the one that is implemented from the other which we call A (i.e. there is a blanket impl<T: A> B for T).
• The A user needs to write its own A wrapper on top of B, because the only way to be generic is to use B.
• When the A user happen to run with an A implementation, we pay the cost of A to B from the blanket impl and the cost of B to A from the user wrapper.
• This is a contradiction. My claim is that the only false assumption is the existence of an order. But maybe I'm wrong and one of those 2 users or one of those 2 implementers does not exist.

My suggestion is to duplicate those 6 functions (or equivalently traits) with one version for implementers only and one version for users only, and provide the blanket impls to connect all implementers to all users (this is not quadratic because there can be blanket impls among those traits that are used transitively and shared).

That's essentially what I was saying in #1311 (comment). And again, I understand that it may be too much complexity and closing as Won't fix is the best course of action.

Regarding inout, that would indeed fix this issue by breaking the first assumption. Because I would expect only one type of implementer: inout detached (which is the union of in-place detached and separate detached). This would be the bottom of the topological order. I don't think any implementations to be optimized for the attached case or the alloc case. Then users can use whatever flavor they want and it will always be implemented through a possibly empty sequence of blanket impls.