RustCrypto / traits

Collection of cryptography-related traits
570 stars 183 forks source link

aead: object safety #70

Closed tarcieri closed 4 years ago

tarcieri commented 4 years ago

Right now the Aead (and other) traits are not object safe, because of the encrypt method's use of impl Into<Payload> and encrypt_in_place method's use of impl Buffer.

I think I'd like to leave the impl Into<Payload> on encrypt as-is, but split Aead and AeadMut into two traits: one object safe, and one not.

Namely I propose we could have AeadInPlace and AeadMutInPlace which are object-safe. The impl Buffer can be replaced with a generic parameter on the trait itself, which would both permit object safety and allow for multiple impls (i.e. for alloc::vec::Vec and heapless::Vec). We could then have a blanket impl of Aead for AeadInPlace<Vec>, and AeadMut for AeadMutInPlace<Vec>.

This would also be helpful for AEAD provider crates who don't want to impl the in-place methods and just have allocating Vec-based APIs, which is the case for ursa: https://github.com/hyperledger/ursa/pull/91

burdges commented 4 years ago

Are separate object safe traits needed, not just object safe methods?

You cannot touch associated types from when being object safe, right? It'd all be slice based?

tarcieri commented 4 years ago

Are separate object safe traits needed, not just object safe methods?

Splitting the traits up this way has additional benefits per the ursa example, so I think it's win-win to get object safety this way.

You cannot touch associated types from when being object safe, right?

You can't use associated types, but you can use a generic parameter on the trait, which is object safe when used with a concrete type.

burdges commented 4 years ago

Is the ursa example really a win? Is there any technical reason to support the Vec only interface? It's better for HSMs or SGX perhaps?

It'll work okay with <Vec<u8>> but generic array parameters sound untenable.

tarcieri commented 4 years ago

@burdges there are several places I've encountered alloc-only APIs. sodiumoxide used to have one. Things which communicate with external encryption services (more typically a KMS) are another.

It'll work okay with <Vec> but generic array parameters sound untenable.

They're part and parcel for using heapless::Vec. I know you hate generic-array, but heapless is widely adopted in the Rust embedded world.

tarcieri commented 4 years ago

Another way to potentially eliminate the generic parameter for aead::Buffer is to use a trait object there too, i.e. &mut dyn Buffer. This would also permit third party impls of aead::Buffer to (continue to) work.

burdges commented 4 years ago

Cool. Yes sodiumoxide, KMS, etc. sound like good reasons. :)

Apologies. All my generic array comment was actually a poorly worded question: How do you handle the nonce and tag size if this stuff will be detached? You do not want three type parameters, right?

It appears your answer is the object safe traits will support in-place operation, but not detached operation. This makes sense. Just me being dense. :)

tarcieri commented 4 years ago

I've opened a PR which splits the alloc vs in-place traits, making the latter object safe here: https://github.com/RustCrypto/traits/pull/120