apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bert Huijben" <b...@qqmail.nl>
Subject RE: svn commit: r1231605 - /apr/apr/trunk/tables/apr_hash.c
Date Sun, 15 Jan 2012 17:06:50 GMT

> -----Original Message-----
> From: bojan@apache.org [mailto:bojan@apache.org]
> Sent: zondag 15 januari 2012 1:37
> To: commits@apr.apache.org
> Subject: svn commit: r1231605 - /apr/apr/trunk/tables/apr_hash.c
> 
> Author: bojan
> Date: Sun Jan 15 00:37:14 2012
> New Revision: 1231605
> 
> URL: http://svn.apache.org/viewvc?rev=1231605&view=rev
> Log:
> Randomise hashes by providing a seed.
> This is supposed to be a "good enough" approach. Using a crypto quality
> function like apr_generate_random_bytes() may be stronger, but this
> function
> may block, so it will be avoided for now.
> 
> 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=123160
> 5&r1=1231604&r2=1231605&view=diff
> ==========================================================
> ====================
> --- apr/apr/trunk/tables/apr_hash.c (original)
> +++ apr/apr/trunk/tables/apr_hash.c Sun Jan 15 00:37:14 2012
> @@ -18,6 +18,10 @@
> 
>  #include "apr_general.h"
>  #include "apr_pools.h"
> +#include "apr_time.h"
> +#if APR_HAVE_STDLIB_H
> +#include <stdlib.h>     /* for rand, srand */
> +#endif
> 
>  #include "apr_hash.h"
> 
> @@ -75,7 +79,7 @@ struct apr_hash_t {
>      apr_pool_t          *pool;
>      apr_hash_entry_t   **array;
>      apr_hash_index_t     iterator;  /* For apr_hash_first(NULL, ...) */
> -    unsigned int         count, max;
> +    unsigned int         count, max, seed;
>      apr_hashfunc_t       hash_func;
>      apr_hash_entry_t    *free;  /* List of recycled entries */
>  };
> @@ -95,13 +99,18 @@ static apr_hash_entry_t **alloc_array(ap
>  APR_DECLARE(apr_hash_t *) apr_hash_make(apr_pool_t *pool)
>  {
>      apr_hash_t *ht;
> +    apr_time_t now = apr_time_now();
> +
>      ht = apr_palloc(pool, sizeof(apr_hash_t));
>      ht->pool = pool;
>      ht->free = NULL;
>      ht->count = 0;
>      ht->max = INITIAL_MAX;
> +    srand((unsigned int)((now >> 32) ^ now ^ (apr_uintptr_t)ht));
> +    ht->seed = (unsigned int)(rand());

If you call srand() before every call to rand() the result is no longer random.

And in this case we do this inside a shared library, so this might introduce other attack
vectors in applications that use apr.

If we really want to call srand() from apr we should do that from a one-time initialization
(apr_initialize? Or some initialize flag), to avoid breaking applications that assume rand()
produces random values for them. Calling srand() from apr_hash_make might even break scientific
applications that want the same set of random values multiple times (srand(CONSTANT)).

If the timer has enough detail we could just use the time, ptr combination as the seed here.
This is essentially what the code does by calling srand() every time anyway.

	Bert


Mime
View raw message