coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 2/2] maint: use getentropy and new tempname modules


From: Paul Eggert
Subject: [PATCH 2/2] maint: use getentropy and new tempname modules
Date: Sun, 31 May 2020 23:33:10 -0700

Update gnulib submodule to latest and use its new features.
Gnulib’s new getentropy module means coreutils can now assume
getentropy instead of approximating it, badly in some cases.
Gnulib’s improvements to the tempname module mean coreutils no
longer needs to maintain private patches.
* bootstrap.conf (gnulib_modules): Remove gettimeofday.
* gl/lib/randread.c (NAME_OF_NONCE_DEVICE): Remove.
(get_nonce): Return success indicator.  Remove bytes_bound arg.
All callers changed.  Rewrite by using getentropy instead of
reading the nonce device and falling back on gettimeofday.
Fail if getentropy fails.
(randread_new): Return NULL (setting errno) if get_nonce fails.
All callers changed.
* gl/lib/tempname.c.diff, gl/lib/tempname.h.diff:
* gl/modules/tempname.diff: Remove.
* gl/modules/randread (Depends-on):
Depend on getentropy, not gettimeofday.
* src/ptx.c (swallow_file_in_memory):
* src/shuf.c (read_input):
Adjust to read_file changes in Gnulib.
* src/shred.c (main):
* src/shuf.c (main):
* src/sort.c (random_md5_state_init):
Diagnose the new form of randread_new failures: randread_new can
fail now when !random_source, meaning getentropy failed.
---
 bootstrap.conf           |   1 -
 gl/lib/randread.c        |  66 +++++---------
 gl/lib/tempname.c.diff   | 189 ---------------------------------------
 gl/lib/tempname.h.diff   |  23 -----
 gl/modules/randread      |   2 +-
 gl/modules/tempname.diff |  13 ---
 gnulib                   |   2 +-
 src/ptx.c                |   4 +-
 src/shred.c              |   3 +-
 src/shuf.c               |   5 +-
 src/sort.c               |   2 +-
 11 files changed, 30 insertions(+), 280 deletions(-)
 delete mode 100644 gl/lib/tempname.c.diff
 delete mode 100644 gl/lib/tempname.h.diff
 delete mode 100644 gl/modules/tempname.diff

diff --git a/bootstrap.conf b/bootstrap.conf
index f34ad6eb5..12e2d831a 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -116,7 +116,6 @@ gnulib_modules="
   getpass-gnu
   gettext-h
   gettime
-  gettimeofday
   getugroups
   getusershell
   git-version-gen
diff --git a/gl/lib/randread.c b/gl/lib/randread.c
index 05d479a8f..c4d3d7410 100644
--- a/gl/lib/randread.c
+++ b/gl/lib/randread.c
@@ -66,10 +66,6 @@
 # define ALIGNED_POINTER(ptr, type) ((size_t) (ptr) % alignof (type) == 0)
 #endif
 
-#ifndef NAME_OF_NONCE_DEVICE
-# define NAME_OF_NONCE_DEVICE "/dev/urandom"
-#endif
-
 /* The maximum buffer size used for reads of random data.  Using the
    value 2 * ISAAC_BYTES makes this the largest power of two that
    would not otherwise cause struct randread_source to grow.  */
@@ -143,52 +139,24 @@ simple_new (FILE *source, void const *handler_arg)
   return s;
 }
 
-/* Put a nonce value into BUFFER, with size BUFSIZE, but do not get
-   more than BYTES_BOUND bytes' worth of random information from any
-   nonce device.  */
+/* Put a nonce value into BUFFER, with size BUFSIZE.
+   Return true on success, false (setting errno) on failure.  */
 
-static void
-get_nonce (void *buffer, size_t bufsize, size_t bytes_bound)
+static bool
+get_nonce (void *buffer, size_t bufsize)
 {
-  char *buf = buffer;
-  ssize_t seeded = 0;
-
-  /* Get some data from FD if available.  */
-  int fd = open (NAME_OF_NONCE_DEVICE, O_RDONLY | O_BINARY);
-  if (0 <= fd)
+  char *buf = buffer, *buflim = buf + bufsize;
+  while (buf < buflim)
     {
-      seeded = read (fd, buf, MIN (bufsize, bytes_bound));
-      if (seeded < 0)
-        seeded = 0;
-      close (fd);
-    }
-
-  /* If there's no nonce device, use a poor approximation
-     by getting the time of day, etc.  */
-#define ISAAC_SEED(type, initialize_v)                      \
-  if (seeded < bufsize)                                     \
-    {                                                       \
-      type v;                                               \
-      size_t nbytes = MIN (sizeof v, bufsize - seeded);     \
-      initialize_v;                                         \
-      memcpy (buf + seeded, &v, nbytes);                    \
-      seeded += nbytes;                                     \
+      int getentropy_bound = 256;
+      int nbytes = MIN (buflim - buf, getentropy_bound);
+      if (getentropy (buf, nbytes) != 0)
+        return false;
+      buf += nbytes;
     }
-  ISAAC_SEED (struct timeval, gettimeofday (&v, NULL));
-  ISAAC_SEED (pid_t, v = getpid ());
-  ISAAC_SEED (pid_t, v = getppid ());
-  ISAAC_SEED (uid_t, v = getuid ());
-  ISAAC_SEED (uid_t, v = getgid ());
-
-#ifdef lint
-  /* Normally we like having the extra randomness from uninitialized
-     parts of BUFFER.  However, omit this randomness if we want to
-     avoid false-positives from memory-checking debugging tools.  */
-  memset (buf + seeded, 0, bufsize - seeded);
-#endif
+  return true;
 }
 
-
 /* Create and initialize a random data source from NAME, or use a
    reasonable default source if NAME is null.  BYTES_BOUND is an upper
    bound on the number of bytes that will be needed.  If zero, it is a
@@ -221,8 +189,14 @@ randread_new (char const *name, size_t bytes_bound)
       else
         {
           s->buf.isaac.buffered = 0;
-          get_nonce (s->buf.isaac.state.m, sizeof s->buf.isaac.state.m,
-                     bytes_bound);
+          if (! get_nonce (s->buf.isaac.state.m,
+                           MIN (sizeof s->buf.isaac.state.m, bytes_bound)))
+            {
+              int e = errno;
+              randread_free (s);
+              errno = e;
+              return NULL;
+            }
           isaac_seed (&s->buf.isaac.state);
         }
 
diff --git a/gl/lib/tempname.c.diff b/gl/lib/tempname.c.diff
deleted file mode 100644
index 43858e97e..000000000
--- a/gl/lib/tempname.c.diff
+++ /dev/null
@@ -1,189 +0,0 @@
-diff --git a/lib/tempname.c b/lib/tempname.c
-index 69c572f..1920274 100644
---- a/lib/tempname.c
-+++ b/lib/tempname.c
-@@ -20,6 +20,7 @@
- #if !_LIBC
- # include <config.h>
- # include "tempname.h"
-+# include "randint.h"
- #endif
-
- #include <sys/types.h>
-@@ -47,6 +48,7 @@
- # error report this to bug-gnulib@gnu.org
- #endif
-
-+#include <stdbool.h>
- #include <stddef.h>
- #include <stdlib.h>
- #include <string.h>
-@@ -173,28 +175,34 @@ __path_search (char *tmpl, size_t tmpl_len, const char 
*dir, const char *pfx,
- }
- #endif /* _LIBC */
-
-+static inline bool _GL_ATTRIBUTE_PURE
-+check_x_suffix (char const *s, size_t len)
-+{
-+  return len <= strspn (s, "X");
-+}
-+
- /* These are the characters used in temporary file names.  */
- static const char letters[] =
- "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
-
- int
--__try_tempname (char *tmpl, int suffixlen, void *args,
--                int (*tryfunc) (char *, void *))
-+try_tempname_len (char *tmpl, int suffixlen, void *args,
-+                  int (*tryfunc) (char *, void *), size_t x_suffix_len)
- {
--  int len;
-+  size_t len;
-   char *XXXXXX;
--  static uint64_t value;
--  uint64_t random_time_bits;
-   unsigned int count;
-   int fd = -1;
-   int save_errno = errno;
-+  struct randint_source *rand_src;
-
-   /* A lower bound on the number of temporary files to attempt to
-      generate.  The maximum total number of temporary file names that
-      can exist for a given template is 62**6.  It should never be
-      necessary to try all of these combinations.  Instead if a reasonable
-      number of names is tried (we define reasonable as 62**3) fail to
--     give the system administrator the chance to remove the problems.  */
-+     give the system administrator the chance to remove the problems.
-+     This value requires that X_SUFFIX_LEN be at least 3.  */
- #define ATTEMPTS_MIN (62 * 62 * 62)
-
-   /* The number of times to attempt to generate a temporary file.  To
-@@ -206,57 +214,55 @@ __try_tempname (char *tmpl, int suffixlen, void *args,
- #endif
-
-   len = strlen (tmpl);
--  if (len < 6 + suffixlen || memcmp (&tmpl[len - 6 - suffixlen], "XXXXXX", 6))
-+  if (len < x_suffix_len + suffixlen
-+      || ! check_x_suffix (&tmpl[len - x_suffix_len - suffixlen],
-+                           x_suffix_len))
-     {
-       __set_errno (EINVAL);
-       return -1;
-     }
-
-   /* This is where the Xs start.  */
--  XXXXXX = &tmpl[len - 6 - suffixlen];
-+  XXXXXX = &tmpl[len - x_suffix_len - suffixlen];
-
-   /* Get some more or less random data.  */
--#ifdef RANDOM_BITS
--  RANDOM_BITS (random_time_bits);
--#else
--  {
--    struct timeval tv;
--    __gettimeofday (&tv, NULL);
--    random_time_bits = ((uint64_t) tv.tv_usec << 16) ^ tv.tv_sec;
--  }
--#endif
--  value += random_time_bits ^ __getpid ();
-+  rand_src = randint_all_new (NULL, x_suffix_len);
-+  if (! rand_src)
-+    return -1;
-
--  for (count = 0; count < attempts; value += 7777, ++count)
-+  for (count = 0; count < attempts; ++count)
-     {
--      uint64_t v = value;
--
--      /* Fill in the random bits.  */
--      XXXXXX[0] = letters[v % 62];
--      v /= 62;
--      XXXXXX[1] = letters[v % 62];
--      v /= 62;
--      XXXXXX[2] = letters[v % 62];
--      v /= 62;
--      XXXXXX[3] = letters[v % 62];
--      v /= 62;
--      XXXXXX[4] = letters[v % 62];
--      v /= 62;
--      XXXXXX[5] = letters[v % 62];
-+      size_t i;
-+
-+      for (i = 0; i < x_suffix_len; i++)
-+        XXXXXX[i] = letters[randint_genmax (rand_src, sizeof letters - 2)];
-
-       fd = tryfunc (tmpl, args);
-       if (fd >= 0)
-         {
-           __set_errno (save_errno);
--          return fd;
-+          goto done;
-         }
-       else if (errno != EEXIST)
--        return -1;
-+        {
-+          fd = -1;
-+          goto done;
-+        }
-     }
-
-+  randint_all_free (rand_src);
-+
-   /* We got out of the loop because we ran out of combinations to try.  */
-   __set_errno (EEXIST);
-   return -1;
-+
-+ done:
-+  {
-+    int saved_errno = errno;
-+    randint_all_free (rand_src);
-+    __set_errno (saved_errno);
-+  }
-+  return fd;
- }
-
- static int
-@@ -285,9 +291,10 @@ try_nocreate (char *tmpl, void *flags _GL_UNUSED)
- }
-
- /* Generate a temporary file name based on TMPL.  TMPL must match the
--   rules for mk[s]temp (i.e. end in "XXXXXX", possibly with a suffix).
-+   rules for mk[s]temp (i.e., end in at least X_SUFFIX_LEN "X"s,
-+   possibly with a suffix).
-    The name constructed does not exist at the time of the call to
--   __gen_tempname.  TMPL is overwritten with the result.
-+   this function.  TMPL is overwritten with the result.
-
-    KIND may be one of:
-    __GT_NOCREATE:       simply verify that the name does not exist
-@@ -298,7 +305,8 @@ try_nocreate (char *tmpl, void *flags _GL_UNUSED)
-
-    We use a clever algorithm to get hard-to-predict names. */
- int
--__gen_tempname (char *tmpl, int suffixlen, int flags, int kind)
-+gen_tempname_len (char *tmpl, int suffixlen, int flags, int kind,
-+                  size_t x_suffix_len)
- {
-   int (*tryfunc) (char *, void *);
-
-@@ -320,5 +328,18 @@ __gen_tempname (char *tmpl, int suffixlen, int flags, int 
kind)
-       assert (! "invalid KIND in __gen_tempname");
-       abort ();
-     }
--  return __try_tempname (tmpl, suffixlen, &flags, tryfunc);
-+  return try_tempname_len (tmpl, suffixlen, &flags, tryfunc, x_suffix_len);
-+}
-+
-+int
-+__gen_tempname (char *tmpl, int suffixlen, int flags, int kind)
-+{
-+  return gen_tempname_len (tmpl, suffixlen, flags, kind, 6);
-+}
-+
-+int
-+__try_tempname (char *tmpl, int suffixlen, void *args,
-+                int (*tryfunc) (char *, void *))
-+{
-+  return try_tempname_len (tmpl, suffixlen, args, tryfunc, 6);
- }
diff --git a/gl/lib/tempname.h.diff b/gl/lib/tempname.h.diff
deleted file mode 100644
index 210e70bd9..000000000
--- a/gl/lib/tempname.h.diff
+++ /dev/null
@@ -1,23 +0,0 @@
-diff --git a/lib/tempname.h b/lib/tempname.h
-index e609360..6029b9f 100644
---- a/lib/tempname.h
-+++ b/lib/tempname.h
-@@ -50,6 +50,8 @@ extern "C" {
-
-    We use a clever algorithm to get hard-to-predict names. */
- extern int gen_tempname (char *tmpl, int suffixlen, int flags, int kind);
-+extern int gen_tempname_len (char *tmpl, int suffixlen, int flags, int kind,
-+                             size_t x_suffix_len);
-
- /* Similar to gen_tempname, but TRYFUNC is called for each temporary
-    name to try.  If TRYFUNC returns a non-negative number, TRY_GEN_TEMPNAME
-@@ -57,6 +59,9 @@ extern int gen_tempname (char *tmpl, int suffixlen, int 
flags, int kind);
-    name is tried, or else TRY_GEN_TEMPNAME returns -1. */
- extern int try_tempname (char *tmpl, int suffixlen, void *args,
-                          int (*tryfunc) (char *, void *));
-+extern int try_tempname_len (char *tmpl, int suffixlen, void *args,
-+                             int (*tryfunc) (char *, void *),
-+                             size_t x_suffix_len);
-
- #ifdef __cplusplus
- }
diff --git a/gl/modules/randread b/gl/modules/randread
index 50e1e0eed..aebe7d962 100644
--- a/gl/modules/randread
+++ b/gl/modules/randread
@@ -12,7 +12,7 @@ error
 exitfail
 inline
 fopen-safer
-gettimeofday
+getentropy
 quote
 stdalign
 stdbool
diff --git a/gl/modules/tempname.diff b/gl/modules/tempname.diff
deleted file mode 100644
index 345f9da02..000000000
--- a/gl/modules/tempname.diff
+++ /dev/null
@@ -1,13 +0,0 @@
-diff --git a/modules/tempname b/modules/tempname
-index 570ea54..f1be8ff 100644
---- a/modules/tempname
-+++ b/modules/tempname
-@@ -13,6 +13,8 @@
- gettimeofday
- lstat
- mkdir
-+randint
-+stdbool
- stdint
- sys_stat
- sys_time
diff --git a/gnulib b/gnulib
index b3c04ecec..8f8da7c80 160000
--- a/gnulib
+++ b/gnulib
@@ -1 +1 @@
-Subproject commit b3c04ecec58ea687423f5c709410e6ecee4abd9b
+Subproject commit 8f8da7c809b6908d074ac64c91d0cedfd4236f2e
diff --git a/src/ptx.c b/src/ptx.c
index 2b9eac7ca..6c88ad287 100644
--- a/src/ptx.c
+++ b/src/ptx.c
@@ -520,9 +520,9 @@ swallow_file_in_memory (const char *file_name, BLOCK *block)
      its name.  */
   bool using_stdin = !file_name || !*file_name || STREQ (file_name, "-");
   if (using_stdin)
-    block->start = fread_file (stdin, &used_length);
+    block->start = fread_file (stdin, 0, &used_length);
   else
-    block->start = read_file (file_name, &used_length);
+    block->start = read_file (file_name, 0, &used_length);
 
   if (!block->start)
     die (EXIT_FAILURE, errno, "%s", quotef (using_stdin ? "-" : file_name));
diff --git a/src/shred.c b/src/shred.c
index fbbeddfbd..d1743501e 100644
--- a/src/shred.c
+++ b/src/shred.c
@@ -1254,7 +1254,8 @@ main (int argc, char **argv)
 
   randint_source = randint_all_new (random_source, SIZE_MAX);
   if (! randint_source)
-    die (EXIT_FAILURE, errno, "%s", quotef (random_source));
+    die (EXIT_FAILURE, errno, "%s",
+         quotef (random_source ? random_source : "getentropy"));
   atexit (clear_random_data);
 
   for (i = 0; i < n_files; i++)
diff --git a/src/shuf.c b/src/shuf.c
index be9be0183..51717ff65 100644
--- a/src/shuf.c
+++ b/src/shuf.c
@@ -275,7 +275,7 @@ read_input (FILE *in, char eolbyte, char ***pline)
      or if none left, stdin.  Doing that would give better performance by
      avoiding the reservoir CPU overhead when reading < RESERVOIR_MIN_INPUT
      from a pipe, and allow us to dispense with the input_size() function.  */
-  if (!(buf = fread_file (in, &used)))
+  if (!(buf = fread_file (in, 0, &used)))
     die (EXIT_FAILURE, errno, _("read error"));
 
   if (used && buf[used - 1] != eolbyte)
@@ -541,7 +541,8 @@ main (int argc, char **argv)
                                      ? SIZE_MAX
                                      : randperm_bound (ahead_lines, n_lines)));
   if (! randint_source)
-    die (EXIT_FAILURE, errno, "%s", quotef (random_source));
+    die (EXIT_FAILURE, errno, "%s",
+         quotef (random_source ? random_source : "getentropy"));
 
   if (use_reservoir_sampling)
     {
diff --git a/src/sort.c b/src/sort.c
index 329ed45dc..d689d58dd 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -2097,7 +2097,7 @@ random_md5_state_init (char const *random_source)
   unsigned char buf[MD5_DIGEST_SIZE];
   struct randread_source *r = randread_new (random_source, sizeof buf);
   if (! r)
-    sort_die (_("open failed"), random_source);
+    sort_die (_("open failed"), random_source ? random_source : "getentropy");
   randread (r, buf, sizeof buf);
   if (randread_free (r) != 0)
     sort_die (_("close failed"), random_source);
-- 
2.17.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]