From 3aa44159c53af1d6459a85dc1841c7c494d52464 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 9 Apr 2012 11:30:46 -0400 Subject: [PATCH] Tweak the new evutil_weakrand_() code Make its state actually get seeded. Document it more thoroughly. Turn its state into a structure. Fix a bug in evutil_weakrand_range_() where it could return the top of the range. Change its return type to ev_int32_t. Add a quick unit test to make sure that the value of evutil_weakrand_range_() is in range. --- bufferevent-internal.h | 4 ++-- bufferevent_ratelim.c | 8 ++++++- event-internal.h | 5 +++-- evutil.c | 49 +++++++++++++++++++++++++++++++++--------- poll.c | 2 ++ select.c | 2 ++ test/regress_buffer.c | 2 +- test/regress_util.c | 9 +++++++- util-internal.h | 29 +++++++++++++++++++++++-- win32select.c | 2 ++ 10 files changed, 93 insertions(+), 19 deletions(-) diff --git a/bufferevent-internal.h b/bufferevent-internal.h index ad2c25c7..461c46e7 100644 --- a/bufferevent-internal.h +++ b/bufferevent-internal.h @@ -105,8 +105,8 @@ struct bufferevent_rate_limit_group { * to refill. */ struct event master_refill_event; - /** Seed for weak random number generator. */ - ev_uint32_t weakrand_seed; + /** Seed for weak random number generator. Protected by 'lock' */ + struct evutil_weakrand_state weakrand_seed; /** Lock to protect the members of this group. This lock should nest * within every bufferevent lock: if you are holding this lock, do diff --git a/bufferevent_ratelim.c b/bufferevent_ratelim.c index 4272c0da..f7de86a9 100644 --- a/bufferevent_ratelim.c +++ b/bufferevent_ratelim.c @@ -438,7 +438,10 @@ bev_refill_callback_(evutil_socket_t fd, short what, void *arg) BEV_UNLOCK(&bev->bev); } -/** Helper: grab a random element from a bufferevent group. */ +/** Helper: grab a random element from a bufferevent group. + * + * Requires that we hold the lock on the group. + */ static struct bufferevent_private * bev_group_random_element_(struct bufferevent_rate_limit_group *group) { @@ -660,6 +663,9 @@ bufferevent_rate_limit_group_new(struct event_base *base, bufferevent_rate_limit_group_set_min_share(g, 64); + evutil_weakrand_seed_(&g->weakrand_seed, + (ev_uint32_t) ((now.tv_sec + now.tv_usec) + (ev_intptr_t)g)); + return g; } diff --git a/event-internal.h b/event-internal.h index 4c882056..78b3fe63 100644 --- a/event-internal.h +++ b/event-internal.h @@ -291,8 +291,9 @@ struct event_base { /** A function used to wake up the main thread from another thread. */ int (*th_notify_fn)(struct event_base *base); - /* Saved seed for weak random number generator. */ - ev_uint32_t weakrand_seed; + /** Saved seed for weak random number generator. Some backends use + * this to produce fairness among sockets. Protected by th_base_lock. */ + struct evutil_weakrand_state weakrand_seed; }; struct event_config_entry { diff --git a/evutil.c b/evutil.c index a87b5246..9307d259 100644 --- a/evutil.c +++ b/evutil.c @@ -2274,21 +2274,50 @@ evutil_getenv_(const char *varname) } ev_uint32_t -evutil_weakrand_(ev_uint32_t* seed) +evutil_weakrand_seed_(struct evutil_weakrand_state *state, ev_uint32_t seed) { - *seed = ((*seed) * 1103515245 + 12345) & 0x7fffffff; - return (*seed); + if (seed == 0) { + struct timeval tv; + evutil_gettimeofday(&tv, NULL); + seed = (ev_uint32_t)tv.tv_sec + (ev_uint32_t)tv.tv_usec; +#ifdef _WIN32 + seed += (ev_uint32_t) _getpid(); +#else + seed += (ev_uint32_t) getpid(); +#endif + } + state->seed = seed; + return seed; } -ev_uint32_t -evutil_weakrand_range_(ev_uint32_t* seed, ev_uint32_t top) +ev_int32_t +evutil_weakrand_(struct evutil_weakrand_state *state) { - ev_uint32_t divisor, result; + /* This RNG implementation is a linear congruential generator, with + * modulus 2^31, multiplier 1103515245, and addend 12345. It's also + * used by OpenBSD, and by Glibc's TYPE_0 RNG. + * + * The linear congruential generator is not an industrial-strength + * RNG! It's fast, but it can have higher-order patterns. Notably, + * the low bits tend to have periodicity. + */ + state->seed = ((state->seed) * 1103515245 + 12345) & 0x7fffffff; + return (ev_int32_t)(state->seed); +} - divisor = EV_INT32_MAX / top; - do - result = evutil_weakrand_(seed) / divisor; - while (result > top); +ev_int32_t +evutil_weakrand_range_(struct evutil_weakrand_state *state, ev_int32_t top) +{ + ev_int32_t divisor, result; + + /* We can't just do weakrand() % top, since the low bits of the LCG + * are less random than the high ones. (Specifically, since the LCG + * modulus is 2^N, every 2^m for m= top); return result; } diff --git a/poll.c b/poll.c index 3ac427ac..01144854 100644 --- a/poll.c +++ b/poll.c @@ -93,6 +93,8 @@ poll_init(struct event_base *base) evsig_init_(base); + evutil_weakrand_seed_(&base->weakrand_seed, 0); + return (pollop); } diff --git a/select.c b/select.c index f9a0c206..8ae53cc1 100644 --- a/select.c +++ b/select.c @@ -121,6 +121,8 @@ select_init(struct event_base *base) evsig_init_(base); + evutil_weakrand_seed_(&base->weakrand_seed, 0); + return (sop); } diff --git a/test/regress_buffer.c b/test/regress_buffer.c index 2e4ef176..d183b4f9 100644 --- a/test/regress_buffer.c +++ b/test/regress_buffer.c @@ -699,7 +699,7 @@ test_evbuffer_add_file(void *ptr) struct event *rev=NULL, *wev=NULL; struct event_base *base = testdata->base; evutil_socket_t pair[2] = {-1, -1}; - static ev_uint32_t seed = 123456789U; + struct evutil_weakrand_state seed = { 123456789U }; /* This test is highly parameterized based on substrings of its * argument. The strings are: */ diff --git a/test/regress_util.c b/test/regress_util.c index 2d53ba8d..6aaeeacf 100644 --- a/test/regress_util.c +++ b/test/regress_util.c @@ -829,7 +829,7 @@ test_evutil_rand(void *arg) char buf2[32]; int counts[256]; int i, j, k, n=0; - static ev_uint32_t seed = 12346789U; + struct evutil_weakrand_state seed = { 12346789U }; memset(buf2, 0, sizeof(buf2)); memset(counts, 0, sizeof(counts)); @@ -869,6 +869,13 @@ test_evutil_rand(void *arg) } } + evutil_weakrand_seed_(&seed, 0); + for (i = 0; i < 10000; ++i) { + ev_int32_t r = evutil_weakrand_range_(&seed, 9999); + tt_int_op(0, <=, r); + tt_int_op(r, <, 9999); + } + /* for (i=0;i<256;++i) { printf("%3d %2d\n", i, counts[i]); } */ end: ; diff --git a/util-internal.h b/util-internal.h index 291feb8a..5248ab21 100644 --- a/util-internal.h +++ b/util-internal.h @@ -254,8 +254,33 @@ int evutil_resolve_(int family, const char *hostname, struct sockaddr *sa, const char *evutil_getenv_(const char *name); -ev_uint32_t evutil_weakrand_(ev_uint32_t* seed); -ev_uint32_t evutil_weakrand_range_(ev_uint32_t* seed, ev_uint32_t top); +/* Structure to hold the state of our weak random number generator. + */ +struct evutil_weakrand_state { + ev_uint32_t seed; +}; + +#define EVUTIL_WEAKRAND_MAX EV_INT32_MAX + +/* Initialize the state of a week random number generator based on 'seed'. If + * the seed is 0, construct a new seed based on not-very-strong platform + * entropy, like the PID and the time of day. + * + * This function, and the other evutil_weakrand* functions, are meant for + * speed, not security or statistical strength. If you need a RNG which an + * attacker can't predict, or which passes strong statistical tests, use the + * evutil_secure_rng* functions instead. + */ +ev_uint32_t evutil_weakrand_seed_(struct evutil_weakrand_state *state, ev_uint32_t seed); +/* Return a pseudorandom value between 0 and EVUTIL_WEAKRAND_MAX inclusive. + * Updates the state in 'seed' as needed -- this value must be protected by a + * lock. + */ +ev_int32_t evutil_weakrand_(struct evutil_weakrand_state *seed); +/* Return a pseudorandom value x such that 0 <= x < top. top must be no more + * than EVUTIL_WEAKRAND_MAX. Updates the state in 'seed' as needed -- this + * value must be proteced by a lock */ +ev_int32_t evutil_weakrand_range_(struct evutil_weakrand_state *seed, ev_int32_t top); /* Evaluates to the same boolean value as 'p', and hints to the compiler that * we expect this value to be false. */ diff --git a/win32select.c b/win32select.c index ce2d525f..7be2389f 100644 --- a/win32select.c +++ b/win32select.c @@ -202,6 +202,8 @@ win32_init(struct event_base *base) if (evsig_init_(base) < 0) winop->signals_are_broken = 1; + evutil_weakrand_seed_(&base->weakrand_seed, 0); + return (winop); err: XFREE(winop->readset_in);