rust-crypto
rust-crypto copied to clipboard
DaGenix
Add Digest::input_hashable method
A method to Digest to allow it to take data of any type that implements std::hash::Hash. Add a default impl for the method.
I'm surprised this didn't exist already which makes me suspect I'm missing some obvious reason why it shouldn't exist. But anyway, I've found this very useful so I'm leaving this PR here as a suggestion.
@DaGenix Bump.
I'm using a local, patched version of rust-crypto that has this for a project that I'm thinking of publishing on crates.io. I don't really wanna have to remove it because it's quite useful.
Would love seeing something like this merged too. (Didn't review the PR yet)
You guys saw the other two open pull requests : https://github.com/DaGenix/rust-crypto/pull/373 https://github.com/DaGenix/rust-crypto/pull/294
If I understand correctly, this pull request allows one to use a cryptographic hash supplied by rust-crypto wherever one might wish to use a regular Rust hash, yes? I think that's okay for security, but..
It'll kill the performance of many tools that expect non-cryptographic hash functions, likely including Rust's HashMap. I have not examined their Robin Hood hashing strategy, but they do keep a u64 hash, so maybe not.
As an example, I'm considering writing a cuckoo hashing based alternative for HashMap that I plan to use to store secret key material indexed by their hashes. It requires a cryptographic has for the public identifier key while the value remains the secret key. It need not store this public identifier key that acts as the key into the hash table, but it does need a small fingerprint of it from which to do it's cuckoo egg thang.
Oh wait. I just saw you have finish returning 0, so it cannot be used at all by data structures. I suppose you're only trying to consume data, not use this hash function. And you want to avoid calling the possibly expensive finish function for the cryptographic hash. I don't suppose it's possible or wise to panic on a call to finish is it?
I don't suppose it's possible or wise to panic on a call to
finishis it?
That's probably a better idea really. The point of this Hasher impl is only to extract data out of a Hash implementer and put it into a Digest. It doesn't make sense to ever call finish. plus DigestHaser is private to the module.
We could write an RFC for a std::HasherIn trait that splits off the final call, so that one does not need to mess with it. Thoughts?
Or maybe someone need to byte the bullet and try to standardize something better than the ByteOrder create for doing that sort of thing.
Edit : I mean, I suppose the underlying issue here is the need to hash larger integer types, but that brings in endianness issues.
Anyways this input_hashable solution looks plenty good enough for any use cases I've noticed. It's necessary for developers to call .to_le() or whatever, as libcore does not do it, but that's fine.
I just opened https://github.com/rust-lang/rfcs/issues/1768 which I believe is relevant.
Just fyi RFC https://github.com/rust-lang/rfcs/pull/1666 would break your input_hashable.
Appears impl<T: Hash> Hash for [T] includes the length so it's not a literal translation.
If T does not contain an [_] itself, then you can avoid this by calling hash_slice. And https://github.com/rust-lang/rfcs/pull/1666 would create a way to patch it recursively.
Also endianness can make a mess of things if one gets sloppy here. It's easy enough to patch if you wrap the Hasher as I mentioned in https://github.com/rust-lang/rfcs/pull/1666#issuecomment-262442138
Just uploaded a quick and dirty wrapper crate to address endiannes in hashing. I suppose it won't work with private Hashers like input_hashable though.