hmac: introduce a helper trait to simplify public bounds

[newpavlov]

Right now public bounds are quite scary:

pub struct HmacCore<D>
where
 D: CoreProxy,
 D::Core: HashMarker
 + UpdateCore
 + FixedOutputCore
 + BufferKindUser<BufferKind = Eager>
 + Default
 + Clone,
 <D::Core as BlockSizeUser>::BlockSize: IsLess<U256>,
 Le<<D::Core as BlockSizeUser>::BlockSize, U256>: NonZero,
{ .. }

Ideally we would use trait aliases, but they are currently not available in stable Rust. As a workaround we can introduce a sealed trait with blanket impl for types which satisfy the current bound and use it in our public bounds.

[newpavlov]

I tried to draft the helper trait, but I can't find a way to prevent the "block size must be less than 256 bytes" bound part from leaking. Ideally it would be encapsulated by the FixedOutputCore trait, but Rust for some reason insists on bubbling it.

Here is one of my attempts:

pub trait EagerHash
where
 Self: CoreProxy,
 Self::EagerCore: HashMarker
 + UpdateCore
 + FixedOutputCore
 + BufferKindUser<BufferKind = Eager>
 + Default
 + Clone,
 <Self::EagerCore as BlockSizeUser>::BlockSize: IsLess<U256>,
 Le<<Self::EagerCore as BlockSizeUser>::BlockSize, U256>: NonZero,
{
 type EagerCore;
}

impl<D> EagerHash for D
where
 D: CoreProxy,
 D::Core: HashMarker
 + UpdateCore
 + FixedOutputCore
 + BufferKindUser<BufferKind = Eager>
 + Default
 + Clone,
 <D::Core as BlockSizeUser>::BlockSize: IsLess<U256>,
 Le<<D::Core as BlockSizeUser>::BlockSize, U256>: NonZero,
{
 type EagerCore = D::Core;
}

pub struct HmacCore<D: EagerHash>
// Leaking bound
where
 <D::EagerCore as BlockSizeUser>::BlockSize: IsLess<U256>,
 Le<<D::EagerCore as BlockSizeUser>::BlockSize, U256>: NonZero,
{
 digest: D::Core,
 opad_digest: D::Core,
 #[cfg(feature = "reset")]
 ipad_digest: D::Core,
}

We could remove the leak by making block-buffer panic on initialization of a buffer with block size bigger than 255 bytes instead of checking this requirement at compile time. It may be even a backwards compatible change...

[tesaguri]

Why doesHmacCore<D> has all of these trait bounds on the type declaration in the first place? It seems that the struct definition only depends on D: CoreProxy and other ones are not technically necessary on the type declaration.

My code has a type that holds a generic Hmac<D> field and the field forces me to replicate the trait bounds on my type declaration and all of its impl blocks, even for implementations that doesn't perform any MAC operations, like Debug. A helper trait would be nice to have, but it would be even greater if I don't need to write these bounds entirely.

[newpavlov]

@tesaguri
Because bounds on D::Core describe the exact atoms of functionality needed by HmacCore. Everything there is used in the implementation.

Until digest v0.11 lands, you could use SimpleHmac. It uses very simple bounds, but a tiny bit less efficient than Hmac.

[tesaguri]

Because bounds on D::Core describe the exact atoms of functionality needed by HmacCore. Everything there is used in the implementation.

I understand they are used in many parts of the implementation, but it's still not everywhere. Debug for HmacCore<D> only depends on D: CoreProxy, D::Core: AlgorithmName in essence, for example. At least, the crate still compiles fine even if you remove the bounds on the type declaration.

Generally speaking, I don't think it's idiomatic for a type declaration to have trait bounds that are not used in its definition. core::iter::Map<I, F>, for example, doesn't have I: Iterator bound on its declaration even though it's the core of the struct's functionality. The bounds on its constructor (in HmacCore's case, bounds on KeyInit implementation are its counterpart) prevent the user from constructing a Map with a wrong I type , so it's still type-safe without bounds on the type declaration (though the typechecker would complain if the user attempts to use it as an iterator anyway).

See also rust-lang/rust-clippy#1689 for further discussions on the matter.

That said, in my use case, the bounds are only affecting an internal type and aren't leaking into user-facing types, so I won't insist on it too much.

[tarcieri]

Another option is to rename SimpleHmac => Hmac and the current Hmac => GenericHmac or something like that.

In most cases it seems like SimpleHmac is what people actually want: simplicity and ease-of-use for relatively short messages.

[newpavlov]

Fixed in #151.