[PATCH] gc-random: Replace implementation with call to getrandom.

[Simon Josefsson]

Hi.  I re-read the discussion around getrandom vs gc-random and didn't
see any point in keeping the duplicated code.  I believe the
getrandom-approach is better than what was in gc-gnulib.c today, so this
patch make it use that function.  I have pushed the patch below.

/Simon

From 44ed0db8c93f6a81fd996f1f10e93051291fbf1d Mon Sep 17 00:00:00 2001
From: Simon Josefsson <simon@josefsson.org>
Date: Wed, 20 Jan 2021 11:50:21 +0100
Subject: [PATCH] gc-random: Replace implementation with call to getrandom.

* lib/gc-gnulib.c [GNULIB_GC_RANDOM]: Replace #include's with
those needed for getrandom.
(gc_init): Remove old randomness code.
(gc_done): Likewise.
(randomize): Rewrite using getrandom, inspired by getentropy.
* m4/gc-random.m4: Remove file.
* modules/crypto/gc-random: Drop gc-random.m4, gl_GC_RANDOM, and
LIB_GC_RANDOM.  Add conditional dependency on getrandom.
* modules/crypto/gc-tests (test_gc_LDADD): Drop LIB_GC_RANDOM.
---
 ChangeLog                |  13 ++++
 lib/gc-gnulib.c          | 138 +++++++--------------------------------
 m4/gc-random.m4          |  89 -------------------------
 modules/crypto/gc-random |   6 +-
 modules/crypto/gc-tests  |   2 +-
 5 files changed, 39 insertions(+), 209 deletions(-)
 delete mode 100644 m4/gc-random.m4

diff --git a/ChangeLog b/ChangeLog
index 92db4972c..0cedb2dc8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2021-01-20  Simon Josefsson  <simon@josefsson.org>
+
+       gc-random: Replace implementation with call to getrandom.
+       * lib/gc-gnulib.c [GNULIB_GC_RANDOM]: Replace #include's with
+       those needed for getrandom.
+       (gc_init): Remove old randomness code.
+       (gc_done): Likewise.
+       (randomize): Rewrite using getrandom, inspired by getentropy.
+       * m4/gc-random.m4: Remove file.
+       * modules/crypto/gc-random: Drop gc-random.m4, gl_GC_RANDOM, and
+       LIB_GC_RANDOM.  Add conditional dependency on getrandom.
+       * modules/crypto/gc-tests (test_gc_LDADD): Drop LIB_GC_RANDOM.
+
 2021-01-20  Bruno Haible  <bruno@clisp.org>
 
        exec*e tests: Avoid test failures on Cygwin.
diff --git a/lib/gc-gnulib.c b/lib/gc-gnulib.c
index 9f361be0e..c6f201bd2 100644
--- a/lib/gc-gnulib.c
+++ b/lib/gc-gnulib.c
@@ -28,11 +28,9 @@
 
 /* For randomize. */
 #if GNULIB_GC_RANDOM
-# include <unistd.h>
-# include <sys/types.h>
-# include <sys/stat.h>
-# include <fcntl.h>
-# include <errno.h>
+#include <unistd.h>
+#include <errno.h>
+#include <sys/random.h>
 #endif
 
 /* Hashes. */
@@ -75,150 +73,62 @@
 # include "rijndael-api-fst.h"
 #endif
 
-#if GNULIB_GC_RANDOM
-# if defined _WIN32 && ! defined __CYGWIN__
-#  include <windows.h>
-#  include <wincrypt.h>
-HCRYPTPROV g_hProv = 0;
-#  ifndef PROV_INTEL_SEC
-#   define PROV_INTEL_SEC 22
-#  endif
-#  ifndef CRYPT_VERIFY_CONTEXT
-#   define CRYPT_VERIFY_CONTEXT 0xF0000000
-#  endif
-# endif
-#endif
-
-#if defined _WIN32 && ! defined __CYGWIN__
-/* Don't assume that UNICODE is not defined.  */
-# undef CryptAcquireContext
-# define CryptAcquireContext CryptAcquireContextA
-#endif
-
 Gc_rc
 gc_init (void)
 {
-#if GNULIB_GC_RANDOM
-# if defined _WIN32 && ! defined __CYGWIN__
-  if (g_hProv)
-    CryptReleaseContext (g_hProv, 0);
-
-  /* There is no need to create a container for just random data, so
-     we can use CRYPT_VERIFY_CONTEXT (one call) see:
-     
https://web.archive.org/web/20070314163712/http://blogs.msdn.com/dangriff/archive/2003/11/19/51709.aspx
 */
-
-  /* We first try to use the Intel PIII RNG if drivers are present */
-  if (!CryptAcquireContext (&g_hProv, NULL, NULL,
-                            PROV_INTEL_SEC, CRYPT_VERIFY_CONTEXT))
-    {
-      /* not a PIII or no drivers available, use default RSA CSP */
-      if (!CryptAcquireContext (&g_hProv, NULL, NULL,
-                                PROV_RSA_FULL, CRYPT_VERIFY_CONTEXT))
-        return GC_RANDOM_ERROR;
-    }
-# endif
-#endif
-
   return GC_OK;
 }
 
 void
 gc_done (void)
 {
-#if GNULIB_GC_RANDOM
-# if defined _WIN32 && ! defined __CYGWIN__
-  if (g_hProv)
-    {
-      CryptReleaseContext (g_hProv, 0);
-      g_hProv = 0;
-    }
-# endif
-#endif
-
   return;
 }
 
 #if GNULIB_GC_RANDOM
 
-/* Randomness. */
-
-static Gc_rc
-randomize (int level, char *data, size_t datalen)
+/* Overwrite BUFFER with random data, under the control of getrandom
+   FLAGS.  BUFFER contains LENGTH bytes.  Inspired by getentropy,
+   however LENGTH is not restricted to 256.  Return 0 on success, -1
+   (setting errno) on failure.  */
+static int
+randomize (void *buffer, size_t length, unsigned int flags)
 {
-#if defined _WIN32 && ! defined __CYGWIN__
-  if (!g_hProv)
-    return GC_RANDOM_ERROR;
-  CryptGenRandom (g_hProv, (DWORD) datalen, data);
-#else
-  int fd;
-  const char *device;
-  size_t len = 0;
-  int rc;
-
-  switch (level)
-    {
-    case 0:
-      device = NAME_OF_NONCE_DEVICE;
-      break;
-
-    case 1:
-      device = NAME_OF_PSEUDO_RANDOM_DEVICE;
-      break;
-
-    default:
-      device = NAME_OF_RANDOM_DEVICE;
-      break;
-    }
-
-  if (strcmp (device, "no") == 0)
-    return GC_RANDOM_ERROR;
+  char *buf = buffer;
 
-  fd = open (device, O_RDONLY | O_CLOEXEC);
-  if (fd < 0)
-    return GC_RANDOM_ERROR;
-
-  do
+  for (;;)
     {
-      ssize_t tmp;
-
-      tmp = read (fd, data, datalen);
-
-      if (tmp < 0)
-        {
-          int save_errno = errno;
-          close (fd);
-          errno = save_errno;
+      ssize_t bytes;
+      if (length == 0)
+        return GC_OK;
+      while ((bytes = getrandom (buf, length, flags)) < 0)
+        if (errno != EINTR)
           return GC_RANDOM_ERROR;
-        }
-
-      len += tmp;
+      if (bytes == 0)
+        break;
+      buf += bytes;
+      length -= bytes;
     }
-  while (len < datalen);
-
-  rc = close (fd);
-  if (rc < 0)
-    return GC_RANDOM_ERROR;
-#endif
 
-  return GC_OK;
+  return GC_RANDOM_ERROR;
 }
 
 Gc_rc
 gc_nonce (char *data, size_t datalen)
 {
-  return randomize (0, data, datalen);
+  return randomize (data, datalen, 0);
 }
 
 Gc_rc
 gc_pseudo_random (char *data, size_t datalen)
 {
-  return randomize (1, data, datalen);
+  return randomize (data, datalen, 0);
 }
 
 Gc_rc
 gc_random (char *data, size_t datalen)
 {
-  return randomize (2, data, datalen);
+  return randomize (data, datalen, GRND_RANDOM);
 }
 
 #endif
diff --git a/m4/gc-random.m4 b/m4/gc-random.m4
deleted file mode 100644
index d27031b27..000000000
--- a/m4/gc-random.m4
+++ /dev/null
@@ -1,89 +0,0 @@
-# gc-random.m4 serial 9
-dnl Copyright (C) 2005-2021 Free Software Foundation, Inc.
-dnl This file is free software; the Free Software Foundation
-dnl gives unlimited permission to copy and/or distribute it,
-dnl with or without modifications, as long as this notice is preserved.
-
-AC_DEFUN([gl_GC_RANDOM],
-[
-  # Devices with randomness.
-  # FIXME: Are these the best defaults?
-
-  AC_REQUIRE([AC_CANONICAL_HOST])
-
-  case "$host_os" in
-    *mirbsd*)
-      NAME_OF_RANDOM_DEVICE="/dev/srandom"
-      NAME_OF_PSEUDO_RANDOM_DEVICE="/dev/prandom"
-      NAME_OF_NONCE_DEVICE="/dev/urandom"
-      ;;
-
-    *irix* | *dec-osf* )
-      NAME_OF_RANDOM_DEVICE="/dev/random"
-      NAME_OF_PSEUDO_RANDOM_DEVICE="/dev/random"
-      NAME_OF_NONCE_DEVICE="/dev/random"
-      ;;
-
-    *)
-      NAME_OF_RANDOM_DEVICE="/dev/random"
-      NAME_OF_PSEUDO_RANDOM_DEVICE="/dev/urandom"
-      NAME_OF_NONCE_DEVICE="/dev/urandom"
-      ;;
-  esac
-
-  AC_MSG_CHECKING([device with (strong) random data...])
-  AC_ARG_ENABLE([random-device],
-        AS_HELP_STRING([--enable-random-device],
-                [device with (strong) randomness (for Nettle)]),
-        NAME_OF_RANDOM_DEVICE=$enableval)
-  AC_MSG_RESULT([$NAME_OF_RANDOM_DEVICE])
-
-  AC_MSG_CHECKING([device with pseudo random data...])
-  AC_ARG_ENABLE([pseudo-random-device],
-        AS_HELP_STRING([--enable-pseudo-random-device],
-                [device with pseudo randomness (for Nettle)]),
-        NAME_OF_PSEUDO_RANDOM_DEVICE=$enableval)
-  AC_MSG_RESULT([$NAME_OF_PSEUDO_RANDOM_DEVICE])
-
-  AC_MSG_CHECKING([device with unpredictable data for nonces...])
-  AC_ARG_ENABLE([nonce-device],
-        AS_HELP_STRING([--enable-nonce-device],
-                [device with unpredictable nonces (for Nettle)]),
-        NAME_OF_NONCE_DEVICE=$enableval)
-  AC_MSG_RESULT([$NAME_OF_NONCE_DEVICE])
-
-  if test "$cross_compiling" != yes; then
-    if test "$NAME_OF_RANDOM_DEVICE" != "no"; then
-      AC_CHECK_FILE([$NAME_OF_RANDOM_DEVICE],,
-        AC_MSG_WARN([[Device '$NAME_OF_RANDOM_DEVICE' does not exist, consider 
to use --enable-random-device]]))
-    fi
-    if test "$NAME_OF_PSEUDO_RANDOM_DEVICE" != "no"; then
-      AC_CHECK_FILE([$NAME_OF_PSEUDO_RANDOM_DEVICE],,
-        AC_MSG_WARN([[Device '$NAME_OF_PSEUDO_RANDOM_DEVICE' does not exist, 
consider to use --enable-pseudo-random-device]]))
-    fi
-    if test "$NAME_OF_NONCE_DEVICE" != "no"; then
-      AC_CHECK_FILE([$NAME_OF_NONCE_DEVICE],,
-        AC_MSG_WARN([[Device '$NAME_OF_NONCE_DEVICE' does not exist, consider 
to use --enable-nonce-device]]))
-    fi
-  else
-    AC_MSG_NOTICE([[Cross compiling, assuming random devices exists on the 
target host...]])
-  fi
-
-  # FIXME?: Open+read 42 bytes+close twice and compare data.  Should differ.
-
-  AC_DEFINE_UNQUOTED([NAME_OF_RANDOM_DEVICE], ["$NAME_OF_RANDOM_DEVICE"],
-                   [defined to the name of the (strong) random device])
-  AC_DEFINE_UNQUOTED([NAME_OF_PSEUDO_RANDOM_DEVICE],
-                         "$NAME_OF_PSEUDO_RANDOM_DEVICE",
-                   [defined to the name of the pseudo random device])
-  AC_DEFINE_UNQUOTED([NAME_OF_NONCE_DEVICE], ["$NAME_OF_NONCE_DEVICE"],
-                   [defined to the name of the unpredictable nonce device])
-
-  case $host_os in
-    mingw*)
-      LIB_GC_RANDOM='-ladvapi32' ;;
-    *)
-      LIB_GC_RANDOM= ;;
-  esac
-  AC_SUBST([LIB_GC_RANDOM])
-])
diff --git a/modules/crypto/gc-random b/modules/crypto/gc-random
index 5158f8576..8a5ae462e 100644
--- a/modules/crypto/gc-random
+++ b/modules/crypto/gc-random
@@ -2,13 +2,12 @@ Description:
 Generic crypto random number functions.
 
 Files:
-m4/gc-random.m4
 
 Depends-on:
 crypto/gc
+getrandom [test "$ac_cv_libgcrypt" != yes]
 
 configure.ac:
-gl_GC_RANDOM
 gl_MODULE_INDICATOR([gc-random])
 
 Makefile.am:
@@ -16,9 +15,6 @@ Makefile.am:
 Include:
 "gc.h"
 
-Link:
-$(LIB_GC_RANDOM)
-
 License:
 LGPLv2+
 
diff --git a/modules/crypto/gc-tests b/modules/crypto/gc-tests
index 214db7a4e..7b153d18d 100644
--- a/modules/crypto/gc-tests
+++ b/modules/crypto/gc-tests
@@ -8,4 +8,4 @@ configure.ac:
 Makefile.am:
 TESTS += test-gc
 check_PROGRAMS += test-gc
-test_gc_LDADD = $(LDADD) @LIB_CRYPTO@ $(LIB_GC_RANDOM)
+test_gc_LDADD = $(LDADD) @LIB_CRYPTO@
-- 
2.20.1

signature.asc
Description: PGP signature