Subject: Re: [PATCH 11/14] rust: add functionality to hash an object

From: "brian m. carlson" <[email-masked]>
Date: Wed, 29 Oct 2025 01:05:30 +0000

On 2025-10-28 at 18:05:59, Ezekiel Newren wrote:
> The name _Hasher_ is already used by std::hash::Hasher. It would be
> preferable to pick a different name to avoid confusion. Perhaps
> CryptoHasher, SecureHasher?

Sure, I can pick a different name if you like.  There are also myriad
`Result` values in Rust: `std::result::Result`, `std::fmt::Result`,
`std::io::Result`, etc., so I don't see a huge problem with it, but as I
said, I can change it if folks prefer.

> I don't understand the point in being able to query whether a given
> hasher is safe or not. How does that change how this hasher code is
> used? If the functions are safe then you wouldn't wrap it in an unsafe
> block. If the functions are declared with unsafe then you'd always
> need to wrap it in an unsafe block whether it's actually safe or not.
> Using unsafe in Rust isn't like error handling where you do something
> different on failure. If something fails in unsafe it's usually
> unrecoverable e.g. segfault due to invalid memory access. My
> understanding of unsafe in Rust means "The compiler can't verify that
> this code is actually safe to run, so I've made sure that it is safe
> myself and I'll let the compiler know what code to ignore during
> compilation."

This is not like `unsafe` in Rust.  We have some SHA-1 functions that
are safe (the default ones) that use SHA-1-DC to detect collisions.
People may also compile their Git version with a faster version of SHA-1
that doesn't detect collisions and that may use hardware acceleration in
cases where we're not dealing with untrusted data.  Taylor benchmarked
it and got some pretty nice performance improvements.

My preference personally was to simply say, "SHA-1 is slow since it's
insecure; use SHA-256 if you want hardware acceleration and good
performance," but my advice was not heeded.

So this allows us to do something like `assert!(hash.is_safe())` in
certain code where we know we have untrusted data to make sure we
haven't been passed a Hasher that has been incorrectly initialized.  We
have some code paths which can accept either (and, depending on which
mode they're operating in, do or don't need a safe hasher), so separate
types are less convenient.  We could do that, however, but it would make
things more complicated and we'd need a trait that covers both.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA