aead: factor apart AeadInPlace/*Detached

[tarcieri]

Factors apart the detached methods of AeadInPlace into a separate AeadInPlaceDetached trait, which itself can now more easily be further refactored (by adding e.g. inout support).

Also adds a PostfixTagged trait which is used to gate the blanket impls.

[tarcieri]

This is the local WIP I was working on to address some of the concerns in #1672 which I mentioned in #1713. It's nowhere close to complete, but I thought I'd push it up for comparison.

[tarcieri]

I wrote this as a stepstone to inout support, with the idea being to replace AeadInPlaceDetached with an AeadInOut trait or thereabouts.

It's a self-contained step in that direction though which is independently useful, however I can open a separate PR that goes to inout support directly (which is really just replacing the input slice with InOutBuf)

[newpavlov]

I still don't think that the trait is better than the IS_POSTFIX: bool associated constant.

Considering that we do not have specialization, it would be hard to implement a generic method encrypt_to_buffer<B: Buffer>(&self, nonce: &Nonce<Self>, aad: &[u8], data: &[u8]) -> Result<B>.

[tarcieri]

A generic implementation of a prefix tag will always be suboptimal due to copying. It requires special features to implement efficiently. It's also an edge case.

[newpavlov]

Note that the method works buffer-to-buffer, so there is no additional suboptimality in it for the prefixed case.

With the constant we can simply write this:

fn encrypt_to_buffer<B: Buffer>(&self, nonce: &Nonce<Self>, aad: &[u8], data: &[u8]) -> Result<B> {
    if Self::IS_POSTFIX {
        // postfix impl
    } else {
        // prefix impl
    }
}

And the compiler will trivially remove the unused branch. It's exactly what I did here (UPD: fixed the link).

Meanwhile, with the trait it's impossible to write such implementation without relying on specialization. Alternatively, you need to introduce an additional hacky PrefixTagged trait.

[tarcieri]

I don't want a PrefixTagged trait.

Personally I am having trouble reading your code. Where is your solution for:

• Accepting the input as a Buffer which contains the plaintext input message
• Resizing the buffer to make room for the tag
• Encrypting the plaintext to ciphertext in-place while shifting the ciphertext forward in the buffer to make room for the prefix tag
• Adding the tag to the beginning of the message

[newpavlov]

Accepting the input as a Buffer which contains the plaintext input message

Here is your fundamental misunderstanding of the proposed method. It accepts data: &[u8] and writes encrypted data into impl Buffer (e.g. Vec). In other words, it fundamentally does not support in-place operation.

The method which implements in-place postfix encryption of a Buffer with resizing for the tag is here. For obvious reasons, we do not have a prefix variant for this.

[newpavlov]

encrypt_to_buffer is the method for "don't care" users. It works with both prefix and postfix AEAD constructions. Yes, it has the inefficiency of writing encrypted data into a separate buffer, but I believe it's a fine tradeoff for "don't care" users.

We could implement a generic in-place encrypt_buffer(&self, aad: &[u8], data: &mut impl Buffer) method in a similar fashion, but it would inevitably involve an overlapping copy inside the buffer in the prefix branch.

[tarcieri]

And this is exactly what the solution in this PR solves:

• There is only one API for end users. The prefix/postfix distinction is not exposed in the methods. Everything Just Works(tm). Exposing it to users just invites them to use the wrong tag order, which would produce AEAD messages incompatible with other implementations, violating standards.
• The postfix case is handled generically
• For the minuscule number of constructions (AES-CMAC-SIV, XSalsa20Poly1305) which do use a prefix construction, those implementations can call directly into an efficient encryption/decryption operation which shifts the output (ring provides such a feature, we don't, but probably should), which could avoid a copy

The latter is what I was talking about at the start of this thread:

A generic implementation of a prefix tag will always be suboptimal due to copying. It requires special features to implement efficiently.

[newpavlov]

There is only one API for end users. The prefix/postfix distinction is not exposed in the methods. Everything Just Works(tm).

And how is it different from the API proposed by me? It also has high level methods which "just work" generically for any construction. Meanwhile with the current design, Aead may not be available for an AEAD implementation since we have a blanket impl only for postfix constructions. It's less "just works" in my opinion compared to my design in which you always have the high-level methods.

Exposing it to users just invites them to use the wrong tag order, which would produce AEAD messages incompatible with other implementations, violating standards.

I disagree with this. We provide a plenty of "use with caution" APIs. The method names literally contain "prefix" and "postfix" in their names and warnings about potential incompatibility with standards in their docs. But I digress, we could gate those methods on IS_POSTFIX, if you really want it that strongly.

For the minuscule number of constructions (AES-CMAC-SIV, XSalsa20Poly1305) which do use a prefix construction, those implementations can call directly into an efficient encryption/decryption operation which shifts the output (ring provides such a feature, we don't, but probably should), which could avoid a copy

Nothing prevents you from overwriting the default method impls in my PR with a more efficient implementation when (if?) we implement the offsetting API.

[tarcieri]

And how is it different from the API proposed by me?

We just went over this: you have no in-place API which abstracts over the prefix/postfix distinction. You instead make users have to care about the distinction.

I don't think we should expose the prefix/postfix distinction to users at all, or at the very least try to make the degree to which we expose it as minimal as possbile.

I disagree with this. We provide a plenty of "use with caution" APIs.

If we have the choice of an API with footguns versus a footgun-free API, we should absolutely choose the footgun-free API.

We sometimes expose APIs with footguns to end users, but with great care (placed under hazmat features/modules), and with clean separation between the two, not forced into the same trait.

[newpavlov]

you have no in-place API which abstracts over the prefix/postfix distinction

And I explicitly wrote that we could easily implement it using overlapping copies for the blanket impl while reserving the ability to overwrite it in future with a more efficient impl.

I would like to return to the PrefixTagged vs IS_POSTFIX discussion, but I guess it's better to open a separate issue for it.

[newpavlov]

I am fine with merging this as a step forward, but I still think that the crate needs a fair amount of work.