Implementing Digest trait to help with generic usage
Closed this issue · 8 comments
The digest crate offers a number of traits to treat all hashers in a generic way. Since FNV is not a crypto-secure hasher, it was understandably rejected by the RustCrypto/hashes project. Yet, I think there is a lot of value to support the same digest traits as other hashers to make it possible to write simple generic code that supports multiple hashers. For example, my nyurik/sqlite-hashes crate uses common code to support several hashing algorithms thanks to this consistency, but I would have to do a lot of custom code to support FNV.
I would like to propose adding a feature-gated digest that would allow FNV to be used as a <T: NamedDigest + Clone + UnwindSafe + RefUnwindSafe + 'static> type.
cc: @HindrikStegenga
Hm. I'm not sure. The trait says "Convenience wrapper trait covering functionality of cryptographic hash functions with fixed output size.", and it seems like a violation of its guarantees. I'm not fully opposed but I don't think it's a good precedent to ignore the crate intent this way.
Feels like digest should ideally be split into a Digest and HashFunction or something with the former having cryptographic properties, and the latter having all the functions.
@Manishearth thx for the reply. I looked into the Digest trait definition, and it does not seem like it has anything "cryptography" related other than the description string itself. So "in theory", if we/they simply ignore/remove the word "cryptographic" from the description you quoted, no other changes (either in code or behavior or anything else) is really needed. Or am I missing something else? My suspicion is that the description was written this way simply because the rest of the crates in that repo are all cryptographic, not because any additional expectation for the hashing strength is expected. Besides, we can even consider MD5 as a non-cryptographic function due to how easy it is to break it - thus really blurring the line between MD5 and FNV and even CRC32.
Yes, if they decide to make the trait not just about cryptographic hashes, I'd be fine with it. I do not want to deal with the issue where someone uses : Digest bound in a way that relies on the cryptographic nature of the hash. I don't understand their users and don't want to muddy the water their, this is a call they should make.
I understand your concern with the wording. My only take on all this is that without a common trait pattern, anyone that wants to support multiple hash algorithms in their code cannot do so without considerable extra code, regardless if the algo is secure cryptographically at the moment, used to be secure, or never was secure at all.
That's why I thought it would make sense to have it as an optional feature-flag-gated feature, and possibly with a clear statement that it does NOT claim to be cryptograhically secure, but does provide the same common hashing interface for a fixed-sized hash generation.
I have written a simple wrapper for all such cases called noncrypto-digests. For now it just implements Fnv, but eventually I plan to add all other ones like CRC32 and the like.
P.S. I was surprised that there is no Clone implemented for the Hasher. Is this by design? I worked around it with the FnvHasher::with_key(hasher.finish()), but that feels like a hack.
I suspect it's an oversight
Thx @Manishearth, implemented in #33