bug#22202: 24.5; SECURITY ISSUE -- Emacs Server vulnerable to random number generator attack on Windows systems

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 743 bytes --]

Eli, thanks for improving the initial seed for (random t) in Emacs. I noticed 
that with this change, my Emacs was opening /dev/urandom twice, because GnuTLS 
does something similar during startup. Also, it was reading more data from 
/dev/urandom than it needed, due to stdio buffering. So I installed the attached 
patch, which defers to GnuTLS and falls back on doing things by hand (without 
stdio) only if GnuTLS is not available or fails. I assume this approach works 
under MS-Windows; if not please let me know and I'll try to fix it.

Would you mind if I removed the newly-added details about current time and 
process ID from the documentation? The idea is that this is internal 
implementation detail that users should not rely on.

[-- Attachment #2: 0001-Prefer-GnuTLS-when-acquiring-random-seed.patch --]
[-- Type: text/x-diff, Size: 4807 bytes --]

From 05e8148a24ebe51fbe758dd16265e8fb81f85953 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 17 Jan 2016 12:12:08 -0800
Subject: [PATCH] Prefer GnuTLS when acquiring random seed

This attempts to improve on the fix for Bug#22202.
* configure.ac (HAVE_DEV_URANDOM): Remove.
Check /dev/urandom existence at run time, not at build time,
since the device could exist in the former but not the latter.
* src/sysdep.c [HAVE_GNUTLS]: Include gnutls/gnutls.h.
(gnutls_rnd) [GNUTLS_VERSION_NUMBER < 0x020c00]: New fallback macro.
(random_seed): New typedef.
(set_random_seed): New static function.
(seed_random): Use them.
(init_random): Use random_seed instead of uintmax_t, so as to
not consume more entropy than needed.  Prefer gnutls_rnd if it
works; this avoids a redundant open of /dev/urandom on
GNU/Linux with modern GnuTLS.
---
 configure.ac | 16 ------------
 src/sysdep.c | 85 ++++++++++++++++++++++++++++++------------------------------
 2 files changed, 43 insertions(+), 58 deletions(-)

diff --git a/configure.ac b/configure.ac
index 6c9b621..8c01aba 100644
--- a/configure.ac
+++ b/configure.ac
@@ -4153,22 +4153,6 @@ fi
 
 AC_TYPE_MBSTATE_T
 
-AC_MSG_CHECKING([whether "/dev/urandom" is available])
-dev_urandom=no
-dnl MSYS, being a Cygwin fork, thinks "/dev/urandom" does exist, so
-dnl don't check this for the MinGW builds.
-if test "${opsys}" != "mingw32"; then
-   if test -r "/dev/urandom"; then
-      AC_DEFINE(HAVE_DEV_URANDOM, 1, [Define if the system supports the "/dev/urandom" device.])
-      dev_urandom=yes
-   fi
-fi
-if test $dev_urandom = yes; then
-   AC_MSG_RESULT(yes)
-else
-   AC_MSG_RESULT(no)
-fi
-
 dnl Fixme: AC_SYS_POSIX_TERMIOS should probably be used, but it's not clear
 dnl how the tty code is related to POSIX and/or other versions of termios.
 dnl The following looks like a useful start.
diff --git a/src/sysdep.c b/src/sysdep.c
index 1fa4229..635443c 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -99,6 +99,15 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "process.h"
 #include "cm.h"
 
+#ifdef HAVE_GNUTLS
+# include <gnutls/gnutls.h>
+#endif
+#if 0x020c00 <= GNUTLS_VERSION_NUMBER
+# include <gnutls/crypto.h>
+#else
+# define gnutls_rnd(level, data, len) (-1)
+#endif
+
 #ifdef WINDOWSNT
 #include <direct.h>
 /* In process.h which conflicts with the local copy.  */
@@ -2068,63 +2077,55 @@ init_signals (bool dumping)
 # endif /* !HAVE_RANDOM */
 #endif /* !RAND_BITS */
 
+#ifdef HAVE_RANDOM
+typedef unsigned int random_seed;
+static void set_random_seed (random_seed arg) { srandom (arg); }
+#elif defined HAVE_LRAND48
+/* Although srand48 uses a long seed, this is unsigned long to avoid
+   undefined behavior on signed integer overflow in init_random.  */
+typedef unsigned long int random_seed;
+static void set_random_seed (random_seed arg) { srand48 (arg); }
+#else
+typedef unsigned int random_seed;
+static void set_random_seed (random_seed arg) { srand (arg); }
+#endif
+
 void
 seed_random (void *seed, ptrdiff_t seed_size)
 {
-#if defined HAVE_RANDOM || ! defined HAVE_LRAND48
-  unsigned int arg = 0;
-#else
-  long int arg = 0;
-#endif
+  random_seed arg = 0;
   unsigned char *argp = (unsigned char *) &arg;
   unsigned char *seedp = seed;
-  ptrdiff_t i;
-  for (i = 0; i < seed_size; i++)
+  for (ptrdiff_t i = 0; i < seed_size; i++)
     argp[i % sizeof arg] ^= seedp[i];
-#ifdef HAVE_RANDOM
-  srandom (arg);
-#else
-# ifdef HAVE_LRAND48
-  srand48 (arg);
-# else
-  srand (arg);
-# endif
-#endif
+  set_random_seed (arg);
 }
 
 void
 init_random (void)
 {
-  uintmax_t v;
-  struct timespec t;
-  bool success = false;
-
-#if HAVE_DEV_URANDOM
-  FILE *fp = fopen ("/dev/urandom", "rb");
-
-  if (fp)
+  random_seed v;
+  if (gnutls_rnd (GNUTLS_RND_NONCE, &v, sizeof v) != 0)
     {
-      int i;
-
-      for (i = 0, v = 0; i < sizeof (uintmax_t); i++)
+      bool success = false;
+#ifndef WINDOWSNT
+      int fd = emacs_open ("/dev/urandom", O_RDONLY | O_BINARY, 0);
+      if (0 <= fd)
 	{
-	  v <<= 8;
-	  v |= fgetc (fp);
+	  success = emacs_read (fd, &v, sizeof v) == sizeof v;
+	  emacs_close (fd);
+	}
+#else
+      success = w32_init_random (&v, sizeof v) == 0;
+#endif
+      if (! success)
+	{
+	  /* Fall back to current time value + PID.  */
+	  struct timespec t = current_timespec ();
+	  v = getpid () ^ t.tv_sec ^ t.tv_nsec;
 	}
-      fclose (fp);
-      success = true;
-    }
-#elif defined WINDOWSNT
-  if (w32_init_random (&v, sizeof v) == 0)
-    success = true;
-#endif	/* HAVE_DEV_URANDOM || WINDOWSNT */
-  if (!success)
-    {
-      /* Fall back to current time value + PID.  */
-      t = current_timespec ();
-      v = getpid () ^ t.tv_sec ^ t.tv_nsec;
     }
-  seed_random (&v, sizeof v);
+  set_random_seed (v);
 }
 
 /*
-- 
2.5.0