Skip to content

aead: Add detached in-place encrypt/decrypt API; make alloc optional #58

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 17, 2019
Merged

aead: Add detached in-place encrypt/decrypt API; make alloc optional #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions aead/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,7 @@ categories = ["cryptography", "no-std"]

[dependencies]
generic-array = { version = "0.12", default-features = false }

[features]
default = ["alloc"]
Copy link
Member

@newpavlov newpavlov Nov 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the implementation crates don't forget to write:

aead = { version = "0.1.2", default-features=false }
[features]
default = ["alloc"]
alloc = ["aead/alloc"]

And in the next minor version we should not forget to clean it up.

alloc = []
132 changes: 127 additions & 5 deletions aead/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,50 @@
#![no_std]

#[cfg(feature = "alloc")]
extern crate alloc;

pub use generic_array;

#[cfg(feature = "alloc")]
use alloc::vec::Vec;
use generic_array::{typenum::Unsigned, ArrayLength, GenericArray};

// Define the default implementation for both `Aead::encrypt()` and
// `AeadMut::encrypt()`. Uses a macro to gloss over `&self` vs `&mut self`.
#[cfg(feature = "alloc")]
macro_rules! encrypt_to_postfix_tagged_vec {
($aead:expr, $nonce:expr, $payload:expr) => {{
let payload = $payload.into();
let mut buffer = Vec::with_capacity(payload.msg.len() + Self::TagSize::to_usize());
buffer.extend_from_slice(payload.msg);

let tag = $aead.encrypt_in_place_detached($nonce, payload.aad, &mut buffer)?;
buffer.extend_from_slice(tag.as_slice());
Ok(buffer)
}};
}

// Define the default implementation for both `Aead::decrypt()` and
// `AeadMut::decrypt()`. Uses a macro to gloss over `&self` vs `&mut self`.
#[cfg(feature = "alloc")]
macro_rules! decrypt_postfix_tagged_ciphertext_to_vec {
($aead:expr, $nonce:expr, $payload:expr) => {{
let payload = $payload.into();

if payload.msg.len() < Self::TagSize::to_usize() {
return Err(Error);
}

let tag_start = payload.msg.len() - Self::TagSize::to_usize();
let mut buffer = Vec::from(&payload.msg[..tag_start]);
let tag = GenericArray::from_slice(&payload.msg[tag_start..]);
$aead.decrypt_in_place_detached($nonce, payload.aad, &mut buffer, tag)?;

Ok(buffer)
}};
}

#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub struct Error;

Expand All @@ -36,7 +73,7 @@ pub trait NewAead {
/// Authenticated Encryption with Associated Data (AEAD) algorithm.
///
/// This trait is intended for use with stateless AEAD algorithms. The
/// `AeadMut` trait provides a stateful interface.
/// [`AeadMut`] trait provides a stateful interface.
pub trait Aead {
/// The length of a nonce.
type NonceSize: ArrayLength<u8>;
Expand Down Expand Up @@ -64,11 +101,27 @@ pub trait Aead {
/// let plaintext = b"Top secret message, handle with care";
/// let ciphertext = cipher.encrypt(nonce, plaintext);
/// ```
///
/// The default implementation assumes a postfix tag (ala AES-GCM,
/// AES-GCM-SIV, ChaCha20Poly1305). [`Aead`] implementations which do not
/// use a postfix tag will need to override this to correctly assemble the
/// ciphertext message.
#[cfg(feature = "alloc")]
fn encrypt<'msg, 'aad>(
&self,
nonce: &GenericArray<u8, Self::NonceSize>,
plaintext: impl Into<Payload<'msg, 'aad>>,
) -> Result<Vec<u8>, Error>;
) -> Result<Vec<u8>, Error> {
encrypt_to_postfix_tagged_vec!(self, nonce, plaintext)
}

/// Encrypt the data in-place, returning the authentication tag
fn encrypt_in_place_detached(
&self,
nonce: &GenericArray<u8, Self::NonceSize>,
associated_data: &[u8],
buffer: &mut [u8],
) -> Result<GenericArray<u8, Self::TagSize>, Error>;

/// Decrypt the given ciphertext slice, and return the resulting plaintext
/// as a vector of bytes.
Expand All @@ -82,11 +135,30 @@ pub trait Aead {
/// let ciphertext = b"...";
/// let plaintext = cipher.decrypt(nonce, ciphertext)?;
/// ```
///
/// The default implementation assumes a postfix tag (ala AES-GCM,
/// AES-GCM-SIV, ChaCha20Poly1305). [`Aead`] implementations which do not
/// use a postfix tag will need to override this to correctly parse the
/// ciphertext message.
#[cfg(feature = "alloc")]
fn decrypt<'msg, 'aad>(
&self,
nonce: &GenericArray<u8, Self::NonceSize>,
ciphertext: impl Into<Payload<'msg, 'aad>>,
) -> Result<Vec<u8>, Error>;
) -> Result<Vec<u8>, Error> {
decrypt_postfix_tagged_ciphertext_to_vec!(self, nonce, ciphertext)
}

/// Decrypt the data in-place, returning an error in the event the provided
/// authentication tag does not match the given ciphertext (i.e. ciphertext
/// is modified/unauthentic)
fn decrypt_in_place_detached(
&self,
nonce: &GenericArray<u8, Self::NonceSize>,
associated_data: &[u8],
buffer: &mut [u8],
tag: &GenericArray<u8, Self::TagSize>,
) -> Result<(), Error>;
}

/// Stateful Authenticated Encryption with Associated Data algorithm.
Expand All @@ -104,22 +176,47 @@ pub trait AeadMut {
///
/// See notes on [`Aead::encrypt()`] about allowable message payloads and
/// Associated Additional Data (AAD).
#[cfg(feature = "alloc")]
fn encrypt<'msg, 'aad>(
&mut self,
nonce: &GenericArray<u8, Self::NonceSize>,
plaintext: impl Into<Payload<'msg, 'aad>>,
) -> Result<Vec<u8>, Error>;
) -> Result<Vec<u8>, Error> {
encrypt_to_postfix_tagged_vec!(self, nonce, plaintext)
}

/// Encrypt the data in-place, returning the authentication tag
fn encrypt_in_place_detached(
&mut self,
nonce: &GenericArray<u8, Self::NonceSize>,
associated_data: &[u8],
buffer: &mut [u8],
) -> Result<GenericArray<u8, Self::TagSize>, Error>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you help me understand why you'd want to return the tag here, rather than doing that in-place as well? Other than requiring split_at_mut() in the macro it seems like it would otherwise be superior...

Copy link
Member Author

@tarcieri tarcieri Oct 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's actually what I originally suggested here: #40 (comment)

@newpavlov suggested returning the tag instead here: #40 (comment)

I ended up liking that suggestion. I think it's cleaner because it's one less side effect, one less mutable argument, and actually leverages the return value.

Every time you rely on a side effect there's a risk it won't happen. That risk is unavoidable with an in-place API: mutation-by-side-effect is the name of the game. But we don't need to rely on it happening it twice, and can instead have the underlying implementation construct the tag by value, instead of it being one more thing for the caller to initialize with the hope the backing implementation actually does its job.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dunno, if the cipher implementation is doing all this in-place, and it has trouble mutating slices, my intuition is that you've probably got bigger problems than worrying about the tag getting written.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @jcape on doing the tag in-place. Typically, the tag will be prefixed/suffixed to the ciphertext anyways. From a performance standpoint, it would be great if this didn't require a memcpy.

IMO, the risk of the backing implementation not mutating the tag is roughly the same as it returning a bad tag.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a bit extreme, but I would be in favor of having 0 visibility into the tag for all APIs. It should be treated as implicit ciphertext expansion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a bit extreme, but I would be in favor of having 0 visibility into the tag for all APIs. It should be treated as implicit ciphertext expansion.

There's an extensive discussion on the ciphertext layout in #40 :-) I'm generally in agreement with you (RFC and all), but if it prevents divergent implementations for weird non-RFC-compliant schemes like the encrypted filesystem thing where the tag lives in some separate metadata structure, I'm OK with exposing this API.


/// Decrypt the given ciphertext slice, and return the resulting plaintext
/// as a vector of bytes.
///
/// See notes on [`Aead::encrypt()`] and [`Aead::decrypt()`] about allowable
/// message payloads and Associated Additional Data (AAD).
#[cfg(feature = "alloc")]
fn decrypt<'msg, 'aad>(
&mut self,
nonce: &GenericArray<u8, Self::NonceSize>,
ciphertext: impl Into<Payload<'msg, 'aad>>,
) -> Result<Vec<u8>, Error>;
) -> Result<Vec<u8>, Error> {
decrypt_postfix_tagged_ciphertext_to_vec!(self, nonce, ciphertext)
}

/// Decrypt the data in-place, returning an error in the event the provided
/// authentication tag does not match the given ciphertext (i.e. ciphertext
/// is modified/unauthentic)
fn decrypt_in_place_detached(
&mut self,
nonce: &GenericArray<u8, Self::NonceSize>,
associated_data: &[u8],
buffer: &mut [u8],
tag: &GenericArray<u8, Self::TagSize>,
) -> Result<(), Error>;
}

/// A blanket implementation of the Stateful AEAD interface for Stateless
Expand All @@ -131,6 +228,7 @@ impl<Algo: Aead> AeadMut for Algo {

/// Encrypt the given plaintext slice, and return the resulting ciphertext
/// as a vector of bytes.
#[cfg(feature = "alloc")]
fn encrypt<'msg, 'aad>(
&mut self,
nonce: &GenericArray<u8, Self::NonceSize>,
Expand All @@ -139,15 +237,39 @@ impl<Algo: Aead> AeadMut for Algo {
<Self as Aead>::encrypt(self, nonce, plaintext)
}

/// Encrypt the data in-place, returning the authentication tag
fn encrypt_in_place_detached(
&mut self,
nonce: &GenericArray<u8, Self::NonceSize>,
associated_data: &[u8],
buffer: &mut [u8],
) -> Result<GenericArray<u8, Self::TagSize>, Error> {
<Self as Aead>::encrypt_in_place_detached(self, nonce, associated_data, buffer)
}

/// Decrypt the given ciphertext slice, and return the resulting plaintext
/// as a vector of bytes.
#[cfg(feature = "alloc")]
fn decrypt<'msg, 'aad>(
&mut self,
nonce: &GenericArray<u8, Self::NonceSize>,
ciphertext: impl Into<Payload<'msg, 'aad>>,
) -> Result<Vec<u8>, Error> {
<Self as Aead>::decrypt(self, nonce, ciphertext)
}

/// Decrypt the data in-place, returning an error in the event the provided
/// authentication tag does not match the given ciphertext (i.e. ciphertext
/// is modified/unauthentic)
fn decrypt_in_place_detached(
&mut self,
nonce: &GenericArray<u8, Self::NonceSize>,
associated_data: &[u8],
buffer: &mut [u8],
tag: &GenericArray<u8, Self::TagSize>,
) -> Result<(), Error> {
<Self as Aead>::decrypt_in_place_detached(self, nonce, associated_data, buffer, tag)
}
}

/// AEAD payloads are a combination of a message (plaintext or ciphertext)
Expand Down