From 6645ad6bdd81e7d5a764e0d94ef52fae053a9e13 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 29 Dec 2018 17:33:27 -0500 Subject: [PATCH] Use a separate random seed for SQL random()/setseed() functions. Previously, the SQL random() function depended on libc's random(3), and setseed() invoked srandom(3). This results in interference between these functions and backend-internal uses of random(3). We'd never paid too much mind to that, but in the wake of commit 88bdbd3f7 which added log_statement_sample_rate, the interference arguably has a security consequence: if log_statement_sample_rate is active then an unprivileged user could probably control which if any of his SQL commands get logged, by issuing setseed() at the right times. That seems bad. To fix this reliably, we need random() and setseed() to use their own private random state variable. Standard random(3) isn't amenable to such usage, so let's switch to pg_erand48(). It's hard to say whether that's more or less "random" than any particular platform's version of random(3), but it does have a wider seed value and a longer period than are required by POSIX, so we can hope that this isn't a big downgrade. Also, we should now have uniform behavior of random() across platforms, which is worth something. While at it, upgrade the per-process seed initialization method to use pg_strong_random() if available, greatly reducing the predictability of the initial seed value. (I'll separately do something similar for the internal uses of random().) In addition to forestalling the possible security problem, this has a benefit in the other direction, which is that we can now document setseed() as guaranteeing a reproducible sequence of random() values. Previously, because of the possibility of internal calls of random(3), we could not promise any such thing. Discussion: https://postgr.es/m/3859.1545849900@sss.pgh.pa.us --- doc/src/sgml/func.sgml | 14 ++++++---- src/backend/utils/adt/float.c | 52 +++++++++++++++++++++++++++++------ 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 37860996a6..90d67f1acf 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -1136,11 +1136,15 @@ - The characteristics of the values returned by - random() depend - on the system implementation. It is not suitable for cryptographic - applications; see module for an alternative. - + The random() function uses a simple linear + congruential algorithm. It is fast but not suitable for cryptographic + applications; see the module for a more + secure alternative. + If setseed() is called, the results of + subsequent random() calls in the current session are + repeatable by re-issuing setseed() with the same + argument. + Finally, shows the diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c index cf9327f885..add099ec9d 100644 --- a/src/backend/utils/adt/float.c +++ b/src/backend/utils/adt/float.c @@ -22,10 +22,13 @@ #include "catalog/pg_type.h" #include "common/int.h" #include "libpq/pqformat.h" +#include "miscadmin.h" #include "utils/array.h" +#include "utils/backend_random.h" #include "utils/float.h" #include "utils/fmgrprotos.h" #include "utils/sortsupport.h" +#include "utils/timestamp.h" /* Configurable GUC parameter */ @@ -53,6 +56,10 @@ float8 degree_c_sixty = 60.0; float8 degree_c_one_half = 0.5; float8 degree_c_one = 1.0; +/* State for drandom() and setseed() */ +static bool drandom_seed_set = false; +static unsigned short drandom_seed[3] = {0, 0, 0}; + /* Local function prototypes */ static double sind_q1(double x); static double cosd_q1(double x); @@ -2378,8 +2385,30 @@ drandom(PG_FUNCTION_ARGS) { float8 result; - /* result [0.0 - 1.0) */ - result = (double) random() / ((double) MAX_RANDOM_VALUE + 1); + /* Initialize random seed, if not done yet in this process */ + if (unlikely(!drandom_seed_set)) + { + /* + * If possible, initialize the seed using high-quality random bits. + * Should that fail for some reason, we fall back on a lower-quality + * seed based on current time and PID. + */ + if (!pg_backend_random((char *) drandom_seed, sizeof(drandom_seed))) + { + TimestampTz now = GetCurrentTimestamp(); + uint64 iseed; + + /* Mix the PID with the most predictable bits of the timestamp */ + iseed = (uint64) now ^ ((uint64) MyProcPid << 32); + drandom_seed[0] = (unsigned short) iseed; + drandom_seed[1] = (unsigned short) (iseed >> 16); + drandom_seed[2] = (unsigned short) (iseed >> 32); + } + drandom_seed_set = true; + } + + /* pg_erand48 produces desired result range [0.0 - 1.0) */ + result = pg_erand48(drandom_seed); PG_RETURN_FLOAT8(result); } @@ -2392,13 +2421,20 @@ Datum setseed(PG_FUNCTION_ARGS) { float8 seed = PG_GETARG_FLOAT8(0); - int iseed; + uint64 iseed; - if (seed < -1 || seed > 1) - elog(ERROR, "setseed parameter %f out of range [-1,1]", seed); - - iseed = (int) (seed * MAX_RANDOM_VALUE); - srandom((unsigned int) iseed); + if (seed < -1 || seed > 1 || isnan(seed)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("setseed parameter %g is out of allowed range [-1,1]", + seed))); + + /* Use sign bit + 47 fractional bits to fill drandom_seed[] */ + iseed = (int64) (seed * (float8) UINT64CONST(0x7FFFFFFFFFFF)); + drandom_seed[0] = (unsigned short) iseed; + drandom_seed[1] = (unsigned short) (iseed >> 16); + drandom_seed[2] = (unsigned short) (iseed >> 32); + drandom_seed_set = true; PG_RETURN_VOID(); } -- 2.40.0