Use a separate random seed for SQL random()/setseed() functions.
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 29 Dec 2018 22:33:27 +0000 (17:33 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 29 Dec 2018 22:33:27 +0000 (17:33 -0500)
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
src/backend/utils/adt/float.c

index 37860996a6e18bff80063a3e18f58e861a37e6ef..90d67f1acfde46fa86cb964cf60b2ae16dd321e0 100644 (file)
    </table>
 
   <para>
-   The characteristics of the values returned by
-   <literal><function>random()</function></literal> depend
-   on the system implementation. It is not suitable for cryptographic
-   applications; see <xref linkend="pgcrypto"/> module for an alternative.
-   </para>
+   The <function>random()</function> function uses a simple linear
+   congruential algorithm.  It is fast but not suitable for cryptographic
+   applications; see the <xref linkend="pgcrypto"/> module for a more
+   secure alternative.
+   If <function>setseed()</function> is called, the results of
+   subsequent <function>random()</function> calls in the current session are
+   repeatable by re-issuing <function>setseed()</function> with the same
+   argument.
+  </para>
 
   <para>
    Finally, <xref linkend="functions-math-trig-table"/> shows the
index cf9327f885325aa27f356b899a8d17c63e0d49ab..add099ec9dca7ee6ee863abc68c8fff43925bfc3 100644 (file)
 #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();
 }