From: Ben Knoble Date: Wed, 29 Oct 2025 16:02:29 GMT Subject: Re: [PATCH 11/14] rust: add functionality to hash an object Message-ID: <6F7696A3-A984-4736-9C2C-D0766443A817@gmail.com> In-Reply-To: > Le 28 oct. 2025 à 21:06, brian m. carlson a écrit : > > 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 > Given the confusion on the names, perhaps some docs in the code helps? Or maybe it’s already doc’d over by the FFI type, in which case a note may suffice— “Safe” here is about the hashing algorithm and (un)trusted data, not Rust memory safety. See XYZ for more details.