On 29.01.2012 13:34, Ruediger Pluem wrote: > > [email protected] wrote: >> Author: bojan >> Date: Sat Jan 28 15:54:22 2012 >> New Revision: 1237078 >> >> URL: http://svn.apache.org/viewvc?rev=1237078&view=rev >> Log: >> Further improve hash randomisation: >> Fix naming of a static function. >> Randomise final hash produced by any hash function, using default hash >> function and seed. >> >> Modified: >> apr/apr/trunk/tables/apr_hash.c >> >> Modified: apr/apr/trunk/tables/apr_hash.c >> URL: >> http://svn.apache.org/viewvc/apr/apr/trunk/tables/apr_hash.c?rev=1237078&r1=1237077&r2=1237078&view=diff >> ============================================================================== >> --- apr/apr/trunk/tables/apr_hash.c (original) >> +++ apr/apr/trunk/tables/apr_hash.c Sat Jan 28 15:54:22 2012 >> @@ -290,11 +289,10 @@ static apr_hash_entry_t **find_entry(apr >> { >> apr_hash_entry_t **hep, *he; >> unsigned int hash; >> + apr_ssize_t hlen = sizeof(hash); >> >> - if (ht->hash_func) >> - hash = ht->hash_func(key, &klen); >> - else >> - hash = apr_hashfunc_default_internal(key, &klen, ht->seed); >> + hash = ht->hash_func(key, &klen); >> + hash = hashfunc_default((char *)&hash, &hlen, ht->seed); > Don't we have the same issue here as with the XOR version of the patch? > If two different keys (key1, key2) result in the same hash value
"same hash value" is less common than "same hash slot," so no, in general, that's not the case. A good, seeded cryptographic hash function would scatter the values much better than this does. However, I hardly think that the default implementation should concern itself with that. If someone needs a more secure hash table, they can always provide their own hash function. It's not as if our randomization is likely to make these hash tables suddenly vastly more secure. It just makes the described attack harder to do, with minimal cost to performance and none to API compatibility. -- Brane
