aead: add explicit nonce API

[tarcieri]

Building towards a full solution to #1666, this adds an initial API which supports explicit nonces, implemented as a prefix to the AEAD message.

Putting the nonce in any other position than the message prefix doesn't make sense. Nothing else works that way. There are multiple possible permutations like putting the nonce between the ciphertext, or at the end, but nobody does that, and the best thing we can do for users is eliminate unnecessary choices.

[tarcieri]

Marked as draft for some initial bikeshedding, because it has no tests, and because the blanket impl of Aead for AeadInOut could be made more efficient

[newpavlov]

This looks fine to me. But I am not quite sure about naming. i think it can confusing to have encrypt and encrypt_with_explicit_nonce with the exactly same API. It may be worth to somehow indicate what the methods include nonce into ciphertext.

Also encrypt_with_explicit_nonce is implemented somewhat inefficiently. The ciphertext allocation is not necessary, we could write directly into zero-initialized out. But I guess we can leave it for a later PR.

It also may be worth to implement these methods generically over the Buffer trait.

[tarcieri]

But I am not quite sure about naming. i think it can confusing to have encrypt and encrypt_with_explicit_nonce with the exactly same API. It may be worth to somehow indicate what the methods include nonce into ciphertext.

That's literally what _with_explicit_nonce is supposed to indicate. I also proposed placing these methods under a separate trait to prevent confusion, but you opposed that. It might still make sense, IMO.

[newpavlov]

I meant something like encrypt_with_included(_random/rng)_nonce. It's quite long, but I don't have anything better right now. Moving the methods to a separate trait will not help with communicating the meaning of these methods in places of their use.

_with_explicit_nonce looks like a method which accepts an explicit nonce (as opposed to an implicitly generated), not a method which includes nonce into ciphertext.

[tarcieri]

The trait name can group logical functionality, like AeadWithNoncePrefix, then it doesn't need to be splatted into method names so obnoxiously long you can't even come up with a name for them

[newpavlov]

I also thought about making encrypt/decrypt methods generic over ciphertext layout. Something like encrypt::<Layout>(..) where Layout can be CtTagNonce, NonceCtTag, Default, NonceDefault, DefaultNonce, etc., where Default is either CtTag or TagCt depending on TAG_KIND.

The trait name can group logical functionality, like AeadWithNoncePrefix, then it doesn't need to be splatted into method names so obnoxiously long you can't even come up with a name for them

The trait name is not present where we use the methods unless you use the fully qualified syntax. In code we have just cipher.encrypt_with_explicit_nonce without any indication from which trait the method came.

[tarcieri]

Reiterating this:

Putting the nonce in any other position than the message prefix doesn't make sense. Nothing else works that way. There are multiple possible permutations like putting the nonce between the ciphertext, or at the end, but nobody does that, and the best thing we can do for users is eliminate unnecessary choices.

Oh god please no we don't need to support arbitrary permutations of orderings of the nonce, ciphertext, and tag. That is the canonical ordering used for explicit messages. Anything else is unnecessarily complex and foisting too much choice onto users.

[newpavlov]

We always can keep the non-default layouts in the hazmat module or gate behind a crate feature. The point is to introduce a type parameter to change ciphertext layout instead of adding a bunch of new methods.

[tarcieri]

Unnecessary complexity for complexity's sake is an antipattern. Let's just support the most commonly used pattern to make it easy for users.

[newpavlov]

Are you absolutely sure that nonces are always prepended and that we will not have users asking us to add similar methods which append nonces (e.g. because it allows a more efficient implementation)?

[tarcieri]

I have never seen a counterexample

[tarcieri]

As a general comment, I think unnecessary choices in cryptographic APIs lead to confusion and incompatibilities, and even if you can find a counterexample, a nonce prefix is the overwhelming preference in other libraries I've seen.

We should give users the most commonly used protocol, and if someone needs to support something weird like a postfix nonce, they can do it themselves.

[tarcieri]

The trait name is not present where we use the methods unless you use the fully qualified syntax. In code we have just cipher.encrypt_with_explicit_nonce without any indication from which trait the method came.

The name is still explicit (literally) and unambiguous in case, but the trait provides further clarity.

encrypt_with_included(_random/rng)_nonce

"explicit nonce" versus "included nonce" is just bikeshedding the name, and IMO "explicit nonce" is the name you'll find in the literature for a nonce concatenated with the ciphertext

[newpavlov]

Fine, you can limit the layout types provided by aead to Ct and CtNonce (or ImplicitNonce and ExplicitNonce). The point is that a generic type parameter may be better than to duplicate all methods for each layout. And it would make it easier and less error prone to implement "weird" variants for downstream users. Though I guess it makes no sense to use the nonce-less layout with the encrypt_with_random_nonce method since the generated nonce would be lost.

"explicit nonce" versus "included nonce" is just bikeshedding the name, and IMO "explicit nonce" is the name you'll find in the literature for a nonce concatenated with the ciphertext

IIUC the name "explicit nonce" primarily comes from TLS. If you insist on this terminology, then I think we should rename the other methods to use "implict" for symmetry.

[tarcieri]

If you insist on this terminology, then I think we should rename the other methods to use "implict" for symmetry.

The existing methods can be used for either purpose, so that doesn't make sense.

[tarcieri]

Also I don't think we should add undue complexity to the existing interface just to accommodate nonce prefixes which are an added protocol on top of the existing methods

[tarcieri]

(This, again, is a problem with trying to lump unrelated concepts/protocols into a single trait, which IMO is the source of confusion)

[newpavlov]

The existing methods can be used for either purpose, so that doesn't make sense.

I meant the existing encrypt/decrypt methods. IIUC they work only with the "implicit nonce" layout.

[tarcieri]

This PR literally implements explicit nonces in terms of that API. Surely you'd agree that explicit nonces can't be implemented in terms of "implicit nonces"?

The existing API leaves handling of nonces to the user. That doesn't de facto mean it's an "implicit nonce". That depends on the usage.

[tarcieri]

Also regarding the name "explicit nonce", I'd say there's ample precedent for that term in the IETF/IRTF literature:

https://www.google.com/search?q=site%3Adatatracker.ietf.org+explicit+nonce+aead&oq=site%3Adatatracker.ietf.org+explicit+nonce+aead&gs_lcrp=EgZjaHJvbWUyBggAEEUYOTIGCAEQRRg60gEJMTEzODZqMGo3qAIAsAIA&sourceid=chrome&ie=UTF-8

[newpavlov]

This PR literally implements explicit nonces in terms of that API. Surely you'd agree that explicit nonces can't be implemented in terms of "implicit nonces"?

This is an internal implementation detail. "Implicit" in this context means that we handle transfer of nonces outside of the ciphertext, Usually, it's a shared algorithm (e.g. counter), but it could be a separate channel as well. You could argue that the latter no longer can be called "implicit", but it's a debatable point.

If your goal is to provide a high-level misuasable and convinient API, then wouldn't it be better for encrypt and decrypt to implement the "explicit nonce" API? Right now, with_explicit_nonce methods look salted, while lower-level (relative to the "explicit" methods) encrypt and decrypt look like recommended go-to methods. You can call the old encrypt method as something like encrypt/decrypt_with_detached_nonce.

[tarcieri]

I'm going to close this.

I don't think it makes sense to put these methods in the same trait as the other methods. It's an implementation of a concrete explicit nonce protocol, whereas the other methods are lower-level and protocol agnostic.

I certainly don't think it makes any sense to go around labeling all of the methods with _implicit_nonce, treating them as some sort of parallel API. Nobody else does that.

I also think it really doesn't make any sense to try to change the definitions of the existing trait methods to start operating on explicit nonces where they didn't before.

I'm going to play around with this some more but implementing the protocol as a struct rather than a trait. That gives us a fresh method namespace where we can use names like encrypt and decrypt for the most commonly used operations.

[newpavlov]

As I wrote in my initial comment, I am fine with the added methods and don't have strong objections. But I have concerns about their relation to the encrypt/decrypt methods. Moving methods into a separate trait will not resolve those.

I think we should answer the following questions:

• What are the recommended go-to methods for unsophisticated users?
• Should the recommended methods be Vec-specific or generic over the Buffer trait?
• Scenarios for which we want to have built-in helper methods and scenarios which we do not want to support explicitly, but users may implement them themselves using lower-level methods.