ChaCha20Poly1305 AEAD

[tarcieri]

Implements the ChaCha20Poly1305 AEAD described in RFC 8439 using the chacha20 and poly1305 crates.

[tarcieri]

Removed the draft PR status on this as it's functionally complete and passing tests, but leaving [WIP] on since it depends on RustCrypto/traits#40 being merged first.

cc @jcape

[tarcieri]

Note: I'd like to do XChaCha20Poly1305 as well, which is a simple extension of this, but I figured I'd start with this as a first pass.

[newpavlov]

I thought in other crates zeroize was optional?

[tarcieri]

Happy to keep it that way if you want. The other crates have a lower MSRV than zeroize, so it had to be optional there. Could always be optional but on-by-default as well.

[newpavlov]

Hm, I guess making it optional and enabling it by default will not make substantial difference compared to the non-optional approach, so we can leave it as-is.

BTW I wonder how zeroize will work in case like this:

struct Foo { .. }

struct Bar { foo: Foo, baz: Baz }

if both Foo and Bar implement Zeroize. Will it zeroize Foo space twice?

[tarcieri]

@newpavlov I've been using explicit Drop handlers, and generally trying to push those down to the "leaf" data owner. If there are two Drop handlers like in your example, they will zero foo twice. I'd suggest only Foo impl Drop in the example above.

[newpavlov]

Looks like it can be made generic over the StreamCipher trait? I wonder if it should not go to the poly1305 crate... Or do you plan to make it even more generic?

[tarcieri]

This is a first cut 😉But yeah, good point.

It would definitely be nice to reuse the same generic "Poly1305 plus a Salsa20 family stream cipher" core to implement Salsa20Poly1305, ChaCha20Poly1305, and XChaCha20Poly1305.

The poly1305 crate might make sense as it's the eponymous algorithm's primary use case. I could also see this further justifying the existence of salsa20-core versus trying to abstract it away into a more generic ctr.

[newpavlov]

BTW I wonder if we should not modify the AEAD trait constructors to new(key, nonce).

[jcape]

BTW I wonder if we should not modify the AEAD trait constructors to new(key, nonce).

That would break things like STREAM that need to pass specific nonce values, wouldn't it?

[tarcieri]

@jcape correct.

Particularly for AES-GCM(-SIV) it'd be nice to do key expansion once for a particular key, and reuse that across nonces, especially when encrypting several AEAD messages in a row as in STREAM (or a transport encryption protocol like TLS or Noise).

It helps less for Salsa20 family ciphers, where you do want to buffer/cache the initial block state (RustCrypto/stream-ciphers#50), but particularly for ChaCha20 w\ 96-bit nonce much of the complexity of computing it comes from the nonce.

[tarcieri]

@newpavlov aside from making the implementation generic, look good for a first cut?

[newpavlov]

Yeap, you can merge it at will.

[tarcieri]

Alright! Now I guess we just gotta figure out what to do about AAD in the aead crate 😉

[newpavlov]

BTW another idea which I had is to introduce a trait for AEAD based on stream ciphers, it would allow us to simplify detached interface. maybe we could even add an associated constant of type enum AdPlace { Beginning, End }, which will be used to assemble message. But I guess it will be better to discuss it in a dedicated issue/PR. :)

[tarcieri]

@newpavlov yeah definitely, almost every AEAD is based on a stream cipher, so it'd be nice to simplify the common case for those

[tarcieri]

Opened an issue to discuss stream cipher-based AEADs: #45