[pgpool-hackers: 4588] Shuffle random functions and use better random numbers
Martijn van Duren
pgpool at list.imperialat.at
Mon May 5 23:36:39 JST 2025
Hello all,
After my previous string diff, this is the final linker warning on
OpenBSD:
ld: warning: pool_pg_utils.c(protocol/pool_pg_utils.o:(select_load_balancing_node)): warning: random() may return deterministic values, is that what you want?
Fumbling around in cryptography/random numbers is never a good idea,
unless you know what you're doing. So I fully expect this diff to be
just a conversation starter.
There's multiple things at play here, but here's my take on the code.
There's 2 places where random numbers are required:
- pool_pg_utils: To select a desired backend
- pool_auth: to generate salts
For pool_pg_utils, the common idiom is:
#if defined(sun) || defined(__sun)
r = (((double) rand()) / RAND_MAX);
#else
r = (((double) random()) / RAND_MAX);
#endif
I think this code contains a bug. According to POSIX[0]:
The rand() function shall compute a sequence of pseudo-random integers
in the range [0, {RAND_MAX}].
...
The value of the {RAND_MAX} macro shall be at least 32767
This means that rand()/RAND_MAX returns a value 0..1.
However, random[1] has:
long integers to return successive pseudo-random numbers in the range
from 0 to (2**31)-1
This means that on systems where RAND_MAX < INT32_MAX the results will
be skewed towards the last backend. This code also duplicates itself
multiple times.
My solution is as followed:
- Remove the defined(sun)/rand() case, since pool_auth already assumes
that random() is available.
- Put the code in its own function as to keep the retrieval of numbers
consistent
- Use a 64 bit random number to make the steps in the resulting double
value as small as possible.
- Use arc4random_buf where available. This is a cryptographically secure
source of random numbers invented on OpenBSD and available in glibc
since 2.36.[2] This function is designed to always return with good
values.
- If arc4random_buf is not available, fall back to RAND_bytes, similar
to what pool_auth does. On LibreSSL based systems this maps to (an
internal copy of) arc4random_buf.
- If RAND_bytes is not available or fails, fall back a modified version
of the PostmasterRandom() code. The changes here are:
- Use getentropy()[2] when available for the seed.
- Since random() supplies 31 bits of entropy, call it 3 times to fill
the entire uint64_t.
For pool_auth, the only usage is random bytes for pool_random, which is
used to fill salts, and thus need to be cryptographically secure.
- When available use arc4random_buf and let pool_random() be a simple
wrapper
- Only build PostmasterRandom when arc4random_buf is not available.
- For PostmasterRandom add an option to get a random_seed via
getentropy[3]
Finally I've removed all other calls to srand{,om}() and their
supporting code/variables. Both random() callers are accompanied with
their own srandom(), and calling srandom() once or twice per process
should suffice.
Sincerely,
Martijn van Duren
[0] https://pubs.opengroup.org/onlinepubs/9699919799/functions/rand.html
[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/random.html
[2] https://www.man7.org/linux/man-pages/man3/arc4random_uniform.3.html
[3] https://pubs.opengroup.org/onlinepubs/9799919799/functions/getentropy.html
diff 8473e1d3d8c80159d1e7cfdfb9444e676d1892c3 24d5cfec984aadc7749734d17dc79c9237cbe7c6
commit - 8473e1d3d8c80159d1e7cfdfb9444e676d1892c3
commit + 24d5cfec984aadc7749734d17dc79c9237cbe7c6
blob - 34f07ef480cb4e478fba00c4f19737c570167f58
blob + 15f359750e4f84bc9709e087514284daabadc182
--- configure.ac
+++ configure.ac
@@ -95,6 +95,7 @@ AC_CHECK_LIB(resolv, main)
AC_CHECK_LIB(crypt, main)
AC_CHECK_FUNCS([strlcat strlcpy])
+AC_CHECK_FUNCS([arc4random_buf getentropy])
dnl Checks for header files.
AC_HEADER_STDC
blob - 862e2f733d4d5165098a1b9c5edeb99ef942402b
blob + 5dfcacfd9408e199eaafa712f91752aa47ef3dbd
--- src/auth/pool_auth.c
+++ src/auth/pool_auth.c
@@ -73,7 +73,9 @@ static int read_password_packet(POOL_CONNECTION * fron
static int send_password_packet(POOL_CONNECTION * backend, int protoMajor, char *password);
static int send_auth_ok(POOL_CONNECTION * frontend, int protoMajor);
static void sendAuthRequest(POOL_CONNECTION * frontend, int protoMajor, int32 auth_req_type, char *extradata, int extralen);
+#ifndef HAVE_ARC4RANDOM_BUF
static long PostmasterRandom(void);
+#endif
static int pg_SASL_continue(POOL_CONNECTION * backend, char *payload, int payloadlen, void *sasl_state, bool final);
static void *pg_SASL_init(POOL_CONNECTION * backend, char *payload, int payloadlen, char *username, char *storedPassword);
@@ -2021,6 +2023,9 @@ send_auth_ok(POOL_CONNECTION * frontend, int protoMajo
void
pool_random(void *buf, size_t len)
{
+#ifdef HAVE_ARC4RANDOM_BUF
+ arc4random_buf(buf, len);
+#else
int ret = 0;
#ifdef USE_SSL
ret = RAND_bytes(buf, len);
@@ -2038,6 +2043,7 @@ pool_random(void *buf, size_t len)
ptr[i] = (rand % 255) + 1;
}
}
+#endif
}
/*
@@ -2049,6 +2055,7 @@ pool_random_salt(char *md5Salt)
pool_random(md5Salt, 4);
}
+#ifndef HAVE_ARC4RANDOM_BUF
/*
* PostmasterRandom
*/
@@ -2063,7 +2070,11 @@ PostmasterRandom(void)
*/
if (random_seed == 0)
{
- do
+#ifdef HAVE_GETENTROPY
+ if (getentropy(&random_seed, sizeof(random_seed)) == -1)
+ random_seed = 0;
+#endif
+ while (random_seed == 0)
{
struct timeval random_stop_time;
@@ -2079,13 +2090,13 @@ PostmasterRandom(void)
((random_stop_time.tv_usec << 16) |
((random_stop_time.tv_usec >> 16) & 0xffff));
}
- while (random_seed == 0);
srandom(random_seed);
}
return random();
}
+#endif
static bool
blob - 35620b9ab47f98d5c6dca795873fad8d2b2d03e9
blob + 890d8089e4d9f2808099c2b8c62514f8f04b4ba0
--- src/pcp_con/pcp_worker.c
+++ src/pcp_con/pcp_worker.c
@@ -112,7 +112,6 @@ pcp_worker_main(int port)
char salt[4];
int random_salt = 0;
- struct timeval uptime;
char tos;
int rsize;
@@ -122,9 +121,6 @@ pcp_worker_main(int port)
/* Identify myself via ps */
init_ps_display("", "", "", "");
- gettimeofday(&uptime, NULL);
- srandom((unsigned int) (getpid() ^ uptime.tv_usec));
-
/* set up signal handlers */
signal(SIGTERM, die);
signal(SIGINT, die);
blob - 53a4cce68dcebc021840f70f9b953d49ca83d488
blob + b647a85978a4690d757f299d862970edc9fb96f6
--- src/protocol/child.c
+++ src/protocol/child.c
@@ -153,8 +153,6 @@ do_child(int *fds)
{
sigjmp_buf local_sigjmp_buf;
POOL_CONNECTION_POOL *volatile backend = NULL;
- struct timeval now;
- struct timezone tz;
/* counter for child_max_connections. "volatile" declaration is necessary
* so that this is counted up even if long jump is issued due to
@@ -214,15 +212,6 @@ do_child(int *fds)
/* Initialize per process context */
pool_init_process_context();
- /* initialize random seed */
- gettimeofday(&now, &tz);
-
-#if defined(sun) || defined(__sun)
- srand((unsigned int) now.tv_usec);
-#else
- srandom((unsigned int) now.tv_usec);
-#endif
-
/* initialize connection pool */
if (pool_init_cp())
{
blob - 25942b6cc0c2a0d6dc2d36f6cf8def7b0aa7de61
blob + a75595d4ccab3a964a327cf8fc9d3dce35d050f4
--- src/protocol/pool_pg_utils.c
+++ src/protocol/pool_pg_utils.c
@@ -23,6 +23,9 @@
#include <arpa/inet.h>
#include <sys/types.h>
#include <unistd.h>
+#ifdef USE_SSL
+#include <openssl/rand.h>
+#endif
#include "protocol/pool_pg_utils.h"
#include "protocol/pool_connection_pool.h"
@@ -43,6 +46,7 @@ static int choose_db_node_id(char *str);
static void free_persistent_db_connection_memory(POOL_CONNECTION_POOL_SLOT * cp);
static void si_enter_critical_region(void);
static void si_leave_critical_region(void);
+static double select_rand_weight(void);
/*
* create a persistent connection
@@ -297,6 +301,63 @@ pool_free_startup_packet(StartupPacket *sp)
}
/*
+ * Select a random value 0..1
+ * Use 64 bit base for maximum spreading.
+ */
+static double
+select_rand_weight(void)
+{
+ uint64_t r;
+#ifdef HAVE_ARC4RANDOM_BUF
+ arc4random_buf(&r, sizeof(r));
+ return ((double)r)/(double)UINT64_MAX;
+#else
+ /* Mixed and matched with PostmasterRandom */
+ extern struct timeval random_start_time;
+ static unsigned int random_seed = 0;
+#ifdef USE_SSL
+ if (RAND_bytes(r, sizeof(r)) == 1)
+ return ((double)r)/(double)UINT64_MAX;
+#endif
+
+ /*
+ * Select a random seed at the time of first receiving a request.
+ */
+ if (random_seed == 0)
+ {
+#ifdef HAVE_GETENTROPY
+ if (getentropy(&random_seed, sizeof(random_seed)) == -1)
+ random_seed = 0;
+#endif
+ while (random_seed == 0)
+ {
+ struct timeval random_stop_time;
+
+ gettimeofday(&random_stop_time, NULL);
+
+ /*
+ * We are not sure how much precision is in tv_usec, so we swap
+ * the high and low 16 bits of 'random_stop_time' and XOR them
+ * with 'random_start_time'. On the off chance that the result is
+ * 0, we loop until it isn't.
+ */
+ random_seed = random_start_time.tv_usec ^
+ ((random_stop_time.tv_usec << 16) |
+ ((random_stop_time.tv_usec >> 16) & 0xffff));
+ }
+
+ srandom(random_seed);
+ }
+#endif
+ /* Random returns up to (2**31)-1 */
+ r = ((uint64_t)random()) << 33;
+ r |= ((uint64_t)random()) << 2;
+ r |= random() & 0x03;
+
+ return ((double)r)/(double)UINT64_MAX;
+}
+
+/*
* Select load balancing node. This function is called when:
* 1) client connects
* 2) the node previously selected for the load balance node is down
@@ -324,11 +385,7 @@ select_load_balancing_node(void)
*/
int suggested_node_id = -2;
-#if defined(sun) || defined(__sun)
- r = (((double) rand()) / RAND_MAX);
-#else
- r = (((double) random()) / RAND_MAX);
-#endif
+ r = select_rand_weight();
/*
* Check user_redirect_preference_list
@@ -490,11 +547,7 @@ select_load_balancing_node(void)
}
}
-#if defined(sun) || defined(__sun)
- r = (((double) rand()) / RAND_MAX) * total_weight;
-#else
- r = (((double) random()) / RAND_MAX) * total_weight;
-#endif
+ r = select_rand_weight() * total_weight;
selected_slot = PRIMARY_NODE_ID;
total_weight = 0.0;
@@ -573,11 +626,7 @@ select_load_balancing_node(void)
}
}
-#if defined(sun) || defined(__sun)
- r = (((double) rand()) / RAND_MAX) * total_weight;
-#else
- r = (((double) random()) / RAND_MAX) * total_weight;
-#endif
+ r = select_rand_weight() * total_weight;
total_weight = 0.0;
for (i = 0; i < NUM_BACKENDS; i++)
@@ -642,11 +691,7 @@ select_load_balancing_node(void)
}
}
-#if defined(sun) || defined(__sun)
- r = (((double) rand()) / RAND_MAX) * total_weight;
-#else
- r = (((double) random()) / RAND_MAX) * total_weight;
-#endif
+ r = select_rand_weight() * total_weight;
selected_slot = PRIMARY_NODE_ID;
More information about the pgpool-hackers
mailing list