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

[Eli Zaretskii]

> Date: Tue, 29 Dec 2015 21:22:55 +0000
> From: Richard Copley <rcopley@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, 22202@debbugs.gnu.org, 
>       Demetri Obenour <demetriobenour@gmail.com>
> 
> I haven't reproduced the whole attack scenario and I don't pretend
> know whether it could work. I don't claim any expertise in software
> security. I just wanted to help out by answering Eli's questions.
> 
> To get back to the OP's main point, given that we already go to the
> trouble of creating this secret, it wouldn't hurt to do it better (on all
> systems, for preference). On Windows it really doesn't seem hard.
> Sorry, no patch, for legal reasons, but there's a simple example on
> the MSDN page for CryptGenRandom.

Can you audit the patch below?  I know next to nothing about
cryptography, and I'm not sure I understood all the flags involved in
these APIs.

Also, generating random numbers with these APIs is significantly
slower -- about 2.5 msec per number, as opposed to something like
175 microsec using 'rand'.  Should we perhaps provide an option
(by default off) to force using the old, weaker, but faster method?

Thanks.

--- src/w32.c~0 2015-11-29 06:48:07.000000000 +0200
+++ src/w32.c   2015-12-30 17:48:19.297251800 +0200
@@ -224,6 +224,8 @@ typedef struct _REPARSE_DATA_BUFFER {
 
 #include <iphlpapi.h>  /* should be after winsock2.h */
 
+#include <wincrypt.h>
+
 #include <c-strcase.h>
 
 #include "w32.h"
@@ -2093,9 +2095,34 @@ init_user_info (void)
     CloseHandle (token);
 }
 
+static HCRYPTPROV w32_crypto_hprov;
+static int
+w32_init_crypt_random (void)
+{
+  if (!CryptAcquireContext (&w32_crypto_hprov, NULL, NULL, PROV_RSA_FULL,
+                           CRYPT_VERIFYCONTEXT | CRYPT_SILENT))
+    {
+      DebPrint (("CryptAcquireContext failed with error %x\n",
+                GetLastError ()));
+      return -1;
+    }
+  return 0;
+}
+
 int
 random (void)
 {
+  if (w32_crypto_hprov)
+    w32_init_crypt_random ();
+  if (w32_crypto_hprov)
+    {
+      const DWORD nbytes = 4;  /* see RAND_BITS in sysdep.c */
+      BYTE rand_buffer[nbytes];
+
+      if (CryptGenRandom (w32_crypto_hprov, nbytes, rand_buffer))
+       return *(int *)&rand_buffer[0];
+    }
+  /* Else fall back on rand ()  */
   /* rand () on NT gives us 15 random bits...hack together 30 bits.  */
   return ((rand () << 15) | rand ());
 }
@@ -2103,6 +2130,18 @@ random (void)
 void
 srandom (int seed)
 {
+  if (!w32_crypto_hprov)
+    w32_init_crypt_random ();
+  if (w32_crypto_hprov)
+    {
+      const DWORD nbytes = 4;  /* see RAND_BITS in sysdep.c */
+      BYTE buf[nbytes];
+
+      memcpy (buf, &seed, sizeof buf);
+      CryptGenRandom (w32_crypto_hprov, nbytes, buf);
+    }
+  /* Always seed rand () as well, in case some future call to
+     CryptGenRandom fails and we need to fall back to rand () */
   srand (seed);
 }
 
@@ -9386,6 +9425,8 @@ globals_of_w32 (void)
   extern void dynlib_reset_last_error (void);
   dynlib_reset_last_error ();
 #endif
+
+  w32_crypto_hprov = (HCRYPTPROV)0;
 }
 
 /* For make-serial-process  */