Duplication / confusion between `KeyInit::new` and `digest::Mac::new`

[tarcieri]

The Mac::new trait is defined as:

 fn new(key: &Key<Self>) -> Self
 where
 Self: KeyInit;

...but it provides no default definition, requires Self impl KeyInit, and causes ambiguity:

error[E0034]: multiple applicable items in scope
 --> aes-siv/src/siv.rs:81:22
 |
81 | let mac = M::new(GenericArray::from_slice(&key[..M::KeySize::to_usize()]));
 | ^^^ multiple `new` found
 |
note: candidate #1 is defined in the trait `KeyInit`
 --> /Users/tony/.cargo/registry/src/github.com-1ecc6299db9ec823/crypto-common-0.1.3/src/lib.rs:115:5
 |
115 | fn new(key: &Key<Self>) -> Self;
 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: candidate #2 is defined in the trait `Mac`
 --> /Users/tony/.cargo/registry/src/github.com-1ecc6299db9ec823/digest-0.10.3/src/mac.rs:21:5
 |
21 | / fn new(key: &Key<Self>) -> Self
22 | | where
23 | | Self: KeyInit;
 | |______________________^
help: disambiguate the associated function for candidate #1

[newpavlov]

Maybe we should we close it as duplicate of #1063 or other way around?

[tarcieri]

Oh whoops, sorry, didn't realize I opened two issues about this

[tarcieri]

It feels like there is a lot of delegation happening through methods-of-the-same name and blanket impls which could be supertrait bounds, across both Digest and Mac.

[newpavlov]

Super-traits make docs less clear, especially for beginners. Rustdoc will not show the new method for the Mac trait, users would have to navigate to the KeyInit trait to find it. I think there was another issue with it, but I can not remember it right now...

Either way, it could be worth to experiment with them in the next breaking release.

[tarcieri]

Yeah sorry, I should open that as a separate issue.

Having KeyInit separate is a good thing, IMO, because it makes it possible to use MAC-related functionality without having direct access to the key, as in HSMs or cryptographic accelerators.

[newpavlov]

Note that you can use the current Mac trait with HSMs as well. The blanket impl exists for Update + FixedOutput + MacMarker and the new method is bound on KeyInit separately.

[tarcieri]

Yeah, I was pointing that out as a drawback of potentially adding a supertrait bound for KeyInit.