On 10/14/2016 03:08 PM, Heikki Linnakangas wrote:
I spent some time whacking that around:
Sigh, forgot attachment. Here you go.
- Heikki
>From 4b3000df3dc71ad41018a6606c92bc4a0adeb8f5 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Fri, 14 Oct 2016 14:58:44 +0300
Subject: [PATCH 1/1] Replace PostmasterRandom() with a stronger way of
generating randomness.
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_60nqgvc6ouk8ygyvk...@mail.gmail.com>
---
src/backend/libpq/auth.c | 21 ++++-
src/backend/postmaster/postmaster.c | 153 ++++++++++--------------------------
src/include/port.h | 3 +
src/port/Makefile | 2 +-
src/port/pg_strong_random.c | 148 ++++++++++++++++++++++++++++++++++
src/tools/msvc/Mkvcbuild.pm | 2 +-
6 files changed, 212 insertions(+), 117 deletions(-)
create mode 100644 src/port/pg_strong_random.c
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 0ba8530..e617ac6 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -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.
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 2d43506..d646692 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -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...
@@ -2344,15 +2334,6 @@ ConnCreate(int serverFd)
}
/*
- * 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
*/
#ifndef EXEC_BACKEND
@@ -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));
@@ -5067,66 +5046,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();
diff --git a/src/include/port.h b/src/include/port.h
index b81fa4a..4bb9fee 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -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);
diff --git a/src/port/Makefile b/src/port/Makefile
index bc9b63a..d34f409 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -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
index 0000000..a404111
--- /dev/null
+++ b/src/port/pg_strong_random.c
@@ -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;
+}
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index de764dd..4d4821a 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -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);
--
2.9.3
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
