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: Add Payload type #48

Merged
merged 1 commit into from
Aug 29, 2019
Merged

aead: Add Payload type #48

merged 1 commit into from
Aug 29, 2019

Conversation

tarcieri
Copy link
Member

This implements a Payload type suggested by @newpavlov here:

#40 (comment)

It uses an impl From<Payload<'msg, 'aad>> for all plaintexts and ciphertexts, with the idea that users of the API who are unconcerned with AAD can just pass the plaintext/ciphertext message they intend to encrypt or decrypt.

With a plaintext: &[u8]:

let ciphertext = cipher.encrypt(nonce, plaintext);
let plaintext = cipher.encrypt(nonce, ciphertext).unwrap();

Or if you do want to pass AAD:

let ciphertext = cipher.encrypt(nonce, Payload { msg: plaintext, aad });
let plaintext = cipher.decrypt(nonce, Payload { msg: ciphertext, aad }).unwrap();

This makes msg and aad explicit when used so users don't accidentally pass their intended plaintexts as aad and accidentally expose them because AAD is unencrypted.

The Into<Payload<'_ '_>> impl on &[u8] automatically uses b"" (i.e. empty byte slice) as the AAD when it's coerced, which is what I think 99% of users will want.

As someone who's concerned about AAD for protocols like Noise (and also other use cases like binding digital signatur keys to ciphertexts), I think for the overwheliming majority of users it's a confusing, superfluous detail. I think this approach neatly leverages Rust's polymorphism to hide this detail in cases where it doesn't matter.

This implements a `Payload` type suggested by @newpavlov here:

#40 (comment)

It uses an `impl From<Payload<'msg, 'aad>>` for all plaintexts and
ciphertexts, with the idea that users of the API who are unconcerned
with AAD can just pass the plaintext/ciphertext message they intend
to encrypt or decrypt.

With a `plaintext: &[u8]`:

```rust
let ciphertext = cipher.encrypt(nonce, plaintext);
let plaintext = cipher.encrypt(nonce, ciphertext).unwrap();
```

Or if you do want to pass AAD:

```rust
let ciphertext = cipher.encrypt(nonce, Payload { msg: plaintext, aad });
let plaintext = cipher.decrypt(nonce, Payload { msg: ciphertext, aad }).unwrap();
```

This makes `msg` and `aad` explicit when used so users don't
accidentally pass their intended plaintexts as `aad` and accidentally
expose them because AAD is unencrypted.

The `Into<Payload<'_ '_>>` impl on `&[u8]` automatically uses `b""`
(i.e. empty byte slice) as the AAD when it's coerced, which is what I
think 99% of users will want.

As someone who's concerned about AAD for protocols like Noise (and also
other use cases like binding digital signatur keys to ciphertexts),
I think for the overwheliming majority of users it's a confusing,
superfluous detail. I think this approach neatly leverages Rust's
polymorphism to hide this detail in cases where it doesn't matter.
@tarcieri
Copy link
Member Author

Note that I don't think this is the be-all-and-end-all API. I think we need a lot more work before we have all of the bases covered.

However, in this PR I'm just looking for something everyone thinks is reasonable enough to ship in a v0.1.0 aead crate so I can actually release my otherwise complete chacha20poly1305 crate.

cc @newpavlov @jcape @isislovecruft

@newpavlov
Copy link
Member

Neat! I really like it how it preserves ergonomics for cases without AD.

@tarcieri
Copy link
Member Author

If there aren't any objections, can we merge this? (I don't have access to this repo)

@newpavlov
Copy link
Member

Ah, yes. I will do v0.1.0 release right away.

@newpavlov newpavlov merged commit e8ce10e into RustCrypto:master Aug 29, 2019
@tarcieri
Copy link
Member Author

@newpavlov I had one more PR I wanted to do first! 😉

@tarcieri tarcieri deleted the aead/payload branch August 29, 2019 14:51
@newpavlov
Copy link
Member

Ok. :) I will do a small commit now with minor edits of Cargo.toml.

dns2utf8 pushed a commit to dns2utf8/traits that referenced this pull request Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants