RustCrypto / traits

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

Can Mac trait be made Trait Object Compatible? #306

Closed mustafapc19 closed 4 years ago

mustafapc19 commented 4 years ago

Right now the methods of Mac trait which returns Output where Self doesnt have Sized trait in it, for getting polymorphism around Mac trait. I am requesting this for this deno issue

tarcieri commented 4 years ago

Can you give a concrete example of where it isn't working and the compiler error?

The main problem is the OutputSize associated type. If you want object safety, you'll have to commit to a particular OutputSize in advance, but otherwise something like this should work:

use crypto_mac::{Mac, consts::U32};

pub type Mac256Box = Box<dyn Mac<OutputSize = U32>>;
mustafapc19 commented 4 years ago

The code I am try to compile :

use wasm_bindgen::prelude::*;
use hmac::{Hmac,  NewMac};
use crypto_mac::{Mac, consts::U32};
use digest::generic_array::{ArrayLength};

#[wasm_bindgen]
pub struct DenoHash{
  inner: Box<dyn  Mac<OutputSize = U32>> ,//where OutputSize: Sized,
//  inner: Box<dyn  Mac<OutputSize = dyn ArrayLength<u8,ArrayType = u8>>> ,//where OutputSize: Sized,
}

Error:

error[E0038]: the trait `crypto_mac::Mac` cannot be made into an object
  --> src/lib.rs:11:3
   |
11 |   inner: Box<dyn  Mac<OutputSize = U32>> ,//where OutputSize: Sized,
   |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `crypto_mac::Mac` cannot be made into an object
   |
   = note: the trait cannot be made into an object because it requires `Self: Sized`

Is the code snippet u suggested compiling in your machine? I was initially trying with i32 then used U32. Same error :(. I am new to trait objects btw :)

tarcieri commented 4 years ago

Aah, the type alias isn't sufficient to check for object safety. It compiles, but you don't get the error until you use it as a concrete type.

It looks like the problem is Mac is bounded on Clone, which is in turn bounded on Sized.

I'm not sure how @newpavlov would feel about removing the Clone bound. It's something I think is easily specified enough as a bound where needed.

mustafapc19 commented 4 years ago

Yeah. U guys decide. I don't have that much rust knowledge to actually comment on this issue. But I do feel that this crate(trait) should support polymorphism. Otherwise I don't see an easy and maintainable way to actually incorporate Hmac with Deno.

burdges commented 4 years ago

As a rule, rust traits must choose their polymorphism style, so anything lower level invariably chooses monomorphisation over trait objects. If you want both polymorphism styles then you either (a) curtail what your trait does or else (b) write two traits like digest::DynDigest or erased_serde.

I'll think dyn Mac could not finalize anyways, only update, so you're maybe discussing a DynMac trait, but MACs are more building blocks, so DynMac does not necessarily make much sense. Are you sure you do not want some DynAead?

I'll caution that modern cryptographic practices discourage algorithm agility for various reasons, which seemingly favors monomorphisation too.

mustafapc19 commented 4 years ago

Yeah. I get your point. I'll try to implement that without polymorphism. :+1: . Closing the issue :)