Replace PostmasterRandom() with a stronger way of generating randomness. scram-random
authorHeikki Linnakangas <[email protected]>
Fri, 14 Oct 2016 11:58:44 +0000 (14:58 +0300)
committerHeikki Linnakangas <[email protected]>
Fri, 14 Oct 2016 11:58:44 +0000 (14:58 +0300)
This adds a new routine, pg_strong_random() for generating random bytes,
for use in both frontend and backend. At the moment, it's only used in
the backend, but the upcoming SCRAM authentication patches need strong
random numbers in libpq as well.

pg_strong_random() can acquire strong random numbers from a number of
sources, depending on what's available:
- OpenSSL RAND_bytes(), if built with OpenSSL
- On Windows, the native cryptographic functions are used
- /dev/urandom
- /dev/random

Original patch by Magnus Hagander, with further tweaks by Michael Paquier
and me.

Discussion: <CAB7nPqRy3krN8quR9XujMVVHYtXJ0_60nqgVc6oUk8ygyVkZsA@mail.gmail.com>

src/backend/libpq/auth.c
src/backend/postmaster/postmaster.c
src/include/port.h
src/port/Makefile
src/port/pg_strong_random.c [new file with mode: 0644]
src/tools/msvc/Mkvcbuild.pm

index 0ba85301149a03be96f127b455d179645b23d95a..e617ac6d4164405f61d598d0d1cb8338a01dee56 100644 (file)
@@ -45,6 +45,12 @@ static void auth_failed(Port *port, int status, char *logdetail);
 static char *recv_password_packet(Port *port);
 static int     recv_and_check_password_packet(Port *port, char **logdetail);
 
+/*----------------------------------------------------------------
+ * MD5 authentication
+ *----------------------------------------------------------------
+ */
+static int CheckMD5Auth(Port *port, char **logdetail);
+
 
 /*----------------------------------------------------------------
  * Ident authentication
@@ -535,9 +541,7 @@ ClientAuthentication(Port *port)
                                ereport(FATAL,
                                                (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
                                                 errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled")));
-                       /* include the salt to use for computing the response */
-                       sendAuthRequest(port, AUTH_REQ_MD5, port->md5Salt, 4);
-                       status = recv_and_check_password_packet(port, &logdetail);
+                       status = CheckMD5Auth(port, &logdetail);
                        break;
 
                case uaPassword:
@@ -696,6 +700,17 @@ recv_password_packet(Port *port)
  *----------------------------------------------------------------
  */
 
+static int
+CheckMD5Auth(Port *port, char **logdetail)
+{
+       /* include the salt to use for computing the response */
+       pg_strong_random(port->md5Salt, sizeof(port->md5Salt));
+
+       sendAuthRequest(port, AUTH_REQ_MD5, port->md5Salt, 4);
+       return recv_and_check_password_packet(port, logdetail);
+}
+
+
 /*
  * Called when we have sent an authorization request for a password.
  * Get the response and check it.
index 2d43506cd0e362fee067fd1c3126fe8a29f6acbc..d64669211c0a72addeb35b9e0e2be689aa560d43 100644 (file)
@@ -358,14 +358,6 @@ static volatile bool avlauncher_needs_signal = false;
 static volatile bool StartWorkerNeeded = true;
 static volatile bool HaveCrashedWorker = false;
 
-/*
- * State for assigning random salts and cancel keys.
- * Also, the global MyCancelKey passes the cancel key assigned to a given
- * backend from the postmaster to that backend (via fork).
- */
-static unsigned int random_seed = 0;
-static struct timeval random_start_time;
-
 #ifdef USE_BONJOUR
 static DNSServiceRef bonjour_sdref = NULL;
 #endif
@@ -403,8 +395,6 @@ static void processCancelRequest(Port *port, void *pkt);
 static int     initMasks(fd_set *rmask);
 static void report_fork_failure_to_client(Port *port, int errnum);
 static CAC_state canAcceptConnections(void);
-static long PostmasterRandom(void);
-static void RandomSalt(char *salt, int len);
 static void signal_child(pid_t pid, int signal);
 static bool SignalSomeChildren(int signal, int targets);
 static void TerminateChildren(int signal);
@@ -579,9 +569,11 @@ PostmasterMain(int argc, char *argv[])
         * Initialize random(3) so we don't get the same values in every run.
         *
         * Note: the seed is pretty predictable from externally-visible facts such
-        * as postmaster start time, so avoid using random() for security-critical
-        * random values during postmaster startup.  At the time of first
-        * connection, PostmasterRandom will select a hopefully-more-random seed.
+        * as postmaster start time, so don't use random() for security-critical
+        * random values (use pg_strong_random() instead).  At the time of first
+        * connection, BackendRun() will select a somewhat-more-random seed, based
+        * on the PID and session start timestamp, but that is still not suitable
+        * for security-critical values.
         */
        srandom((unsigned int) (MyProcPid ^ MyStartTime));
 
@@ -1292,8 +1284,6 @@ PostmasterMain(int argc, char *argv[])
         * Remember postmaster startup time
         */
        PgStartTime = GetCurrentTimestamp();
-       /* PostmasterRandom wants its own copy */
-       gettimeofday(&random_start_time, NULL);
 
        /*
         * We're ready to rock and roll...
@@ -2343,15 +2333,6 @@ ConnCreate(int serverFd)
                return NULL;
        }
 
-       /*
-        * Precompute password salt values to use for this connection. It's
-        * slightly annoying to do this long in advance of knowing whether we'll
-        * need 'em or not, but we must do the random() calls before we fork, not
-        * after.  Else the postmaster's random sequence won't get advanced, and
-        * all backends would end up using the same salt...
-        */
-       RandomSalt(port->md5Salt, sizeof(port->md5Salt));
-
        /*
         * Allocate GSSAPI specific state struct
         */
@@ -3904,7 +3885,12 @@ BackendStartup(Port *port)
         * backend will have its own copy in the forked-off process' value of
         * MyCancelKey, so that it can transmit the key to the frontend.
         */
-       MyCancelKey = PostmasterRandom();
+       if (!pg_strong_random(&MyCancelKey, sizeof(MyCancelKey)))
+       {
+               ereport(LOG,
+                               (errmsg("could not generate random query cancel key")));
+               return STATUS_ERROR;
+       }
        bn->cancel_key = MyCancelKey;
 
        /* Pass down canAcceptConnections state */
@@ -4212,13 +4198,6 @@ BackendRun(Port *port)
        int                     usecs;
        int                     i;
 
-       /*
-        * Don't want backend to be able to see the postmaster random number
-        * generator state.  We have to clobber the static random_seed *and* start
-        * a new random sequence in the random() library function.
-        */
-       random_seed = 0;
-       random_start_time.tv_usec = 0;
        /* slightly hacky way to convert timestamptz into integers */
        TimestampDifference(0, port->SessionStartTime, &secs, &usecs);
        srandom((unsigned int) (MyProcPid ^ (usecs << 12) ^ secs));
@@ -5066,66 +5045,6 @@ StartupPacketTimeoutHandler(void)
 }
 
 
-/*
- * RandomSalt
- */
-static void
-RandomSalt(char *salt, int len)
-{
-       long            rand;
-       int                     i;
-
-       /*
-        * We use % 255, sacrificing one possible byte value, so as to ensure that
-        * all bits of the random() value participate in the result. While at it,
-        * add one to avoid generating any null bytes.
-        */
-       for (i = 0; i < len; i++)
-       {
-               rand = PostmasterRandom();
-               salt[i] = (rand % 255) + 1;
-       }
-}
-
-/*
- * PostmasterRandom
- *
- * Caution: use this only for values needed during connection-request
- * processing.  Otherwise, the intended property of having an unpredictable
- * delay between random_start_time and random_stop_time will be broken.
- */
-static long
-PostmasterRandom(void)
-{
-       /*
-        * Select a random seed at the time of first receiving a request.
-        */
-       if (random_seed == 0)
-       {
-               do
-               {
-                       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));
-               }
-               while (random_seed == 0);
-
-               srandom(random_seed);
-       }
-
-       return random();
-}
-
 /*
  * Count up number of child processes of specified types (dead_end chidren
  * are always excluded).
@@ -5303,31 +5222,37 @@ StartAutovacuumWorker(void)
                         * we'd better have something random in the field to prevent
                         * unfriendly people from sending cancels to them.
                         */
-                       MyCancelKey = PostmasterRandom();
-                       bn->cancel_key = MyCancelKey;
+                       if (pg_strong_random(&MyCancelKey, sizeof(MyCancelKey)))
+                       {
+                               bn->cancel_key = MyCancelKey;
 
-                       /* Autovac workers are not dead_end and need a child slot */
-                       bn->dead_end = false;
-                       bn->child_slot = MyPMChildSlot = AssignPostmasterChildSlot();
-                       bn->bgworker_notify = false;
+                               /* Autovac workers are not dead_end and need a child slot */
+                               bn->dead_end = false;
+                               bn->child_slot = MyPMChildSlot = AssignPostmasterChildSlot();
+                               bn->bgworker_notify = false;
 
-                       bn->pid = StartAutoVacWorker();
-                       if (bn->pid > 0)
-                       {
-                               bn->bkend_type = BACKEND_TYPE_AUTOVAC;
-                               dlist_push_head(&BackendList, &bn->elem);
+                               bn->pid = StartAutoVacWorker();
+                               if (bn->pid > 0)
+                               {
+                                       bn->bkend_type = BACKEND_TYPE_AUTOVAC;
+                                       dlist_push_head(&BackendList, &bn->elem);
 #ifdef EXEC_BACKEND
-                               ShmemBackendArrayAdd(bn);
+                                       ShmemBackendArrayAdd(bn);
 #endif
-                               /* all OK */
-                               return;
+                                       /* all OK */
+                                       return;
+                               }
+
+                               /*
+                                * fork failed, fall through to report -- actual error message was
+                                * logged by StartAutoVacWorker
+                                */
+                               (void) ReleasePostmasterChildSlot(bn->child_slot);
                        }
+                       else
+                               ereport(LOG,
+                                               (errmsg("could not generate random query cancel key")));
 
-                       /*
-                        * fork failed, fall through to report -- actual error message was
-                        * logged by StartAutoVacWorker
-                        */
-                       (void) ReleasePostmasterChildSlot(bn->child_slot);
                        free(bn);
                }
                else
@@ -5615,7 +5540,11 @@ assign_backendlist_entry(RegisteredBgWorker *rw)
         * have something random in the field to prevent unfriendly people from
         * sending cancels to them.
         */
-       MyCancelKey = PostmasterRandom();
+       if (!pg_strong_random(&MyCancelKey, sizeof(MyCancelKey)))
+       {
+               rw->rw_crashed_at = GetCurrentTimestamp();
+               return false;
+       }
        bn->cancel_key = MyCancelKey;
 
        bn->child_slot = MyPMChildSlot = AssignPostmasterChildSlot();
index b81fa4a89eb1130c6431bb41c80ee58198df893d..4bb9feeb019fd0a58c08a4451639046176522c16 100644 (file)
@@ -454,6 +454,9 @@ extern int  pg_codepage_to_encoding(UINT cp);
 extern char *inet_net_ntop(int af, const void *src, int bits,
                          char *dst, size_t size);
 
+/* port/pg_strong_random.c */
+extern bool pg_strong_random(void *buf, size_t len);
+
 /* port/pgcheckdir.c */
 extern int     pg_check_dir(const char *dir);
 
index bc9b63add0479459d146550c0338e1a22a478400..d34f409ee976be87439263cd7c93801f43304250 100644 (file)
@@ -32,7 +32,7 @@ LIBS += $(PTHREAD_LIBS)
 
 OBJS = $(LIBOBJS) $(PG_CRC32C_OBJS) chklocale.o erand48.o inet_net_ntop.o \
        noblock.o path.o pgcheckdir.o pgmkdirp.o pgsleep.o \
-       pgstrcasecmp.o pqsignal.o \
+       pg_strong_random.o pgstrcasecmp.o pqsignal.o \
        qsort.o qsort_arg.o quotes.o sprompt.o tar.o thread.o
 
 # foo_srv.o and foo.o are both built from foo.c, but only foo.o has -DFRONTEND
diff --git a/src/port/pg_strong_random.c b/src/port/pg_strong_random.c
new file mode 100644 (file)
index 0000000..a404111
--- /dev/null
@@ -0,0 +1,148 @@
+/*-------------------------------------------------------------------------
+ *
+ * pg_strong_random.c
+ *       pg_strong_random() function to return a strong random number
+ *
+ * Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group
+ *
+ *
+ * IDENTIFICATION
+ *       src/port/pg_strong_random.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#ifndef FRONTEND
+#include "postgres.h"
+#else
+#include "postgres_fe.h"
+#endif
+
+#include <fcntl.h>
+#include <unistd.h>
+
+#ifdef USE_SSL
+#include <openssl/rand.h>
+#endif
+#ifdef WIN32
+#include <Wincrypt.h>
+#endif
+
+static bool random_from_file(char *filename, void *buf, size_t len);
+
+#ifdef WIN32
+/*
+ * Cache a global crypto provider that only gets freed when the process
+ * exits, in case we need random numbers more than once.
+ */
+static HCRYPTPROV hProvider = 0;
+#endif
+
+/*
+ * Read (random) bytes from a file.
+ */
+static bool
+random_from_file(char *filename, void *buf, size_t len)
+{
+       int                     f;
+       char       *p = buf;
+       ssize_t         res;
+
+       f = open(filename, O_RDONLY, 0);
+       if (f == -1)
+               return false;
+
+       while (len)
+       {
+               res = read(f, p, len);
+               if (res <= 0)
+               {
+                       if (errno == EINTR)
+                               continue;               /* interrupted by signal, just retry */
+
+                       close(f);
+                       return false;
+               }
+
+               p += res;
+               len -= res;
+       }
+
+       close(f);
+       return true;
+}
+
+/*
+ * pg_strong_random
+ *
+ * Generate requested number of random bytes. The bytes are
+ * cryptographically strong random, suitable for use e.g. in key
+ * generation.
+ *
+ * The bytes can be acquired from a number of sources, depending
+ * on what's available. We try the following, in this order:
+ *
+ * 1. OpenSSL's RAND_bytes()
+ * 2. Windows' CryptGenRandom() function
+ * 3. /dev/urandom
+ * 4. /dev/random
+ *
+ * Returns true on success, and false if none of the sources
+ * were available. NB: It is important to check the return value!
+ * Proceeding with key generation when no random data was available
+ * would lead to predictable keys and security issues.
+ */
+bool
+pg_strong_random(void *buf, size_t len)
+{
+#ifdef USE_SSL
+
+       /*
+        * When built with OpenSSL, first try the random generation function from
+        * there.
+        */
+       if (RAND_bytes(buf, len) == 1)
+               return true;
+#endif
+
+#ifdef WIN32
+
+       /*
+        * Windows has CryptoAPI for strong cryptographic numbers.
+        */
+       if (hProvider == 0)
+       {
+               if (!CryptAcquireContext(&hProvider,
+                                                                NULL,
+                                                                MS_DEF_PROV,
+                                                                PROV_RSA_FULL,
+                                                                CRYPT_VERIFYCONTEXT | CRYPT_SILENT))
+               {
+                       /*
+                        * On failure, set back to 0 in case the value was for some reason
+                        * modified.
+                        */
+                       hProvider = 0;
+               }
+       }
+
+       /* Re-check in case we just retrieved the provider */
+       if (hProvider != 0)
+       {
+               if (CryptGenRandom(hProvider, len, buf))
+                       return true;
+       }
+#endif
+
+       /*
+        * If there is no OpenSSL and no CryptoAPI (or they didn't work), then
+        * fall back on reading /dev/urandom or even /dev/random.
+        */
+       if (random_from_file("/dev/urandom", buf, len))
+               return true;
+       if (random_from_file("/dev/random", buf, len))
+               return true;
+
+       /* None of the sources were available. */
+       return false;
+}
index de764dd4d44b28a37e18357f1ce99544a43cceff..4d4821a7c8b847fd429f3e914cdd8e9f28c53052 100644 (file)
@@ -92,7 +92,7 @@ sub mkvcbuild
          srandom.c getaddrinfo.c gettimeofday.c inet_net_ntop.c kill.c open.c
          erand48.c snprintf.c strlcat.c strlcpy.c dirmod.c noblock.c path.c
          pgcheckdir.c pgmkdirp.c pgsleep.c pgstrcasecmp.c pqsignal.c
-         mkdtemp.c qsort.c qsort_arg.c quotes.c system.c
+         mkdtemp.c pg_strong_random.c qsort.c qsort_arg.c quotes.c system.c
          sprompt.c tar.c thread.c getopt.c getopt_long.c dirent.c
          win32env.c win32error.c win32security.c win32setlocale.c);