On Fri, Dec 13, 2024, 19:55 Peter Dimov via Boost
Antony Polukhin wrote:
CONDITIONAL ACCEPT. The behavior of the default constructor must be secure.
Hi Antony, thanks for the review.
I of course agree that HashDOS is a problem. The library documentation points this out in several places, that's why SipHash is included and almost no other non-crypto hashes are, and its use is recommended.
However, I don't agree that the default constructor of the hash algorithm _concept_ is the correct place to attempt to make things secure (or in practice, less insecure than before.)
First, this would place the responsibility on hash algorithm authors, which doesn't seem correct, or reliable.
Second, hash algorithms aren't only used in hash tables, and injecting nondeterminism in their default constructors seems user-hostile. The hash algorithm adaptor, once it's supplied by the library, would be a better place to do that, if we decide to do it.
After Vinnie's comment I've realised my source of confusion. Boost.Hash library is about hashing for unordered containers, but Boost.Hash2 is about hashing algorithms. Hash2 is not a replacement for Boost.Hash, it has different goals. Does it make sense to change the library name to highlight that? Boost.HashAlgorithms? Or there is a plan to go further and provide adapters for hash algorithms usage with unordered containers? Third, using a single process-wide seed is not good practice (as you
yourself observe.) It makes things more secure compared to unseeded use, but not really secure. The correct approach is to use a random seed (preferably of size 192 bits or more) that varies per connection, or per container.
Those 192 bits seem to be related to a particular hashing algorithm. Looks like it makes sense for the hashing algorithm implementor to provide some information on seed length. Is there any plan to get that information from algorithm?