httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Lescohier <daniel.lescoh...@cbsi.com>
Subject Re: [PATCH] mod_unique_id: use ap_random_insecure_bytes() to get unique ID
Date Wed, 26 Jun 2013 13:36:23 GMT
We have an internal custom Apache module that generates UUIDs; it's using a
legacy format that we need for our application.  A few months ago, I was
upgrading it to Apache 2.4, and I worked on modifying the UUID generator
function, so that some of the bytes are random (including replacing the 4
byte IPv4 address element of the uuid that we used before with random data
instead).

When I looked into the ap random functions, I didn't like the
implementation, because I didn't see anywhere in the httpd codebase that
entropy is periodically added to the entropy pool.  After reading the
details of how the Linux entropy pool works (https://lwn
.net/Articles/525204/), I decided to use /dev/urandom instead, since Linux
is periodically adding entropy to it.  This code is not portable, but this
was for a private Apache module that is only used on Linux.

To preserve entropy on the web server machine, I also only generate a
random number once per apache child, then increment an uint32 portion of it
for each unique id call.  I also have seconds and microseconds, so that's
why I think it's OK to do increments from the random base, instead of
generating a new random id on each request.

Our app requires network order time_t in the second 32-bit word, so that's
why the function is like below.  If our app didn't require time_t, then I
would not have used the high order bits of time_t, because it's fairly
constant: to have a more unique id, it would be better to have more random
data instead of that fairly constant data.

/**
* Make a new unique id as follows:
* 1. Define the id.
* 2. Base64-encode the id.
* @param r Apache's request record.
* @return The base64-encode cookie string, allocated in the request pool.
*/
static char *make_id( request_rec *r )
{
#define RAW_COOKIE_SIZE (14)
#define WANT_BASE64_COOKIE_SIZE (19)
#define BASE64_ENCODE_LEN(len) (((len + 2) / 3 * 4) + 1)
#define MASK_BITS(n) ((1<<n) - 1)

    struct {
        apr_uint32_t i1; /* b31: 0, b30-20: random, b19-0: usec; network o
*/
        apr_uint32_t i2; /* seconds, network order */
        apr_uint32_t i3; /* random, host order */
        apr_uint16_t s1; /* random, host order */
    } raw_cookie;
    apr_uint32_t i, rnd;
    char * const b64_cookie = apr_palloc(
        r->pool, BASE64_ENCODE_LEN(RAW_COOKIE_SIZE));

    ap_assert(b64_cookie);
    if (unlikely(! rand_initialized)) {
        /* We delay initialization as late as possible in order
         * to collect more entropy in the random pool. */
        init_random(r);
    }

    rnd = apr_atomic_inc32(&rand_uints.i1);
    i = rnd & MASK_BITS(11);
    i <<= 20;
    i += apr_time_usec(r->request_time);
    raw_cookie.i1 = htonl(i);
    raw_cookie.s1 = rnd >> 11;
    raw_cookie.i2 = htonl(apr_time_sec(r->request_time));
    raw_cookie.i3 = rand_uints.i2;

    apr_base64_encode_binary(b64_cookie, (const void *)&raw_cookie,
        RAW_COOKIE_SIZE);
    if (BASE64_ENCODE_LEN(RAW_COOKIE_SIZE) > WANT_BASE64_COOKIE_SIZE)
        b64_cookie[WANT_BASE64_COOKIE_SIZE] = '\0';
    return b64_cookie;
}

And supporting code:

typedef struct {
    apr_uint32_t i1;
    apr_uint32_t i2;
} rand_uints_t;
static rand_uints_t rand_uints;
static apr_thread_mutex_t *rand_mutex;
static int rand_initialized;

static void init_random(request_rec *r)
{
    ap_assert(APR_SUCCESS == apr_thread_mutex_lock(rand_mutex));
    if (rand_initialized == 0) { /* we won the race to initialize */
        apr_file_t *f;
        apr_size_t bytes_read;
        ap_assert(APR_SUCCESS == apr_file_open(&f, "/dev/urandom",
            APR_READ|APR_BINARY, 0444, r->pool));
        ap_assert(APR_SUCCESS == apr_file_read_full(f, &rand_uints,
            sizeof(rand_uints_t), &bytes_read));
        ap_assert(sizeof(rand_uints) == bytes_read);
        ap_assert(APR_SUCCESS == apr_file_close(f));
        rand_initialized = 1;
    }
    ap_assert(APR_SUCCESS == apr_thread_mutex_unlock(rand_mutex));
}


/**
* Initialization for each child process.
*/
static void init_child(apr_pool_t *p, server_rec *s) {
    ap_assert(APR_SUCCESS == apr_thread_mutex_create(&rand_mutex,
        APR_THREAD_MUTEX_DEFAULT, p));
}



On Wed, Jun 26, 2013 at 8:49 AM, Jan Kalu┼ża <jkaluza@redhat.com> wrote:

> Hi,
>
> currently mod_unique_id uses apr_gethostname(...) and PID pair as a base
> to generate unique ID. The way how it's implemented brings some problems:
>
> 1. For IPv6-only hosts it uses low-order bits of IPv6 address as if they
> were unique, which is wrong.
>
> 2. It relies on working DNS. It can happen that hostname does not have the
> IP assigned during httpd start (for example during the boot) and I think it
> is still valid use-case (without mod_unique_id module loaded, httpd works
> well in this case).
>
> 3. It calls 1 second sleep to overcome possible usage of the same PID
> after restart as the one used before the restart.
>
> If I'm right, we could fix the problems above by using
> ap_random_insecure_bytes instead of "in_addr"/"pid" pair as a base for
> unique ID generation. It would also make the code simpler. I think the
> randomness generated by ap_random_insecure_bytes() is at least the same as
> the one introduced by apr_gethostname() + pid pair.
>
> The attached patch implements it by removing in_addr/pid fields
> unique_id_rec struct and introduces new "root" field which is initialized
> using ap_random_insecure_bytes() function and is used as a base for unique
> IDs.
>
> Regards,
> Jan Kaluza
>

Mime
View raw message