public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Paul Eggert <eggert@cs.ucla.edu>
To: libc-alpha@gnu.org
Subject: [PATCH 3/3] Merge tempname ASLR etc. patch from Gnulib
Date: Mon, 22 Aug 2022 15:58:34 -0500	[thread overview]
Message-ID: <20220822205834.563590-3-eggert@cs.ucla.edu> (raw)
In-Reply-To: <20220822205834.563590-1-eggert@cs.ucla.edu>

Merge patch from Gnulib that fixes problems with ASLR info leak
and entropy loss.  This syncs tempname.c with Gnulib commit
564b523fe97a8d63493aa68acb627b8c40744fb9 (2022-08-22) and
fixes <https://bugs.gnu.org/57129> which was reported against
Emacs and uncovered some unlikely Glibc bugs.
* sysdeps/posix/tempname.c: Don't include stdalign.h.
(mix_random_values): New function.
(random_bits): Use it.  Args are now new value address and
old value, and this function now returns a success indicator.
Omit old USE_GETRANDOM argument: always try getrandom now, as
there is no good reason not to now that GRND_NONBLOCK is used.
Caller changed.  Use CLOCK_REALTIME for for ersatz entropy,
as CLOCK_MONOTONIC doesn't work on some platforms.
Also, mix in ersatz entropy from tv_sec and from clock ().
(try_tempname_len): Do not mix in ASLR-based entropy, as
the result is published to the world and ASLR should be private.
Do not try to use a static var as that has issues if multithreaded.
Instead, simply generate new random bits.
Worry about bias only with high-quality random bits.
---
 sysdeps/posix/tempname.c | 94 +++++++++++++++++++++++++---------------
 1 file changed, 59 insertions(+), 35 deletions(-)

diff --git a/sysdeps/posix/tempname.c b/sysdeps/posix/tempname.c
index 60f8541085..0e2f29f5de 100644
--- a/sysdeps/posix/tempname.c
+++ b/sysdeps/posix/tempname.c
@@ -48,7 +48,6 @@
 #include <string.h>
 
 #include <fcntl.h>
-#include <stdalign.h>
 #include <stdint.h>
 #include <sys/random.h>
 #include <sys/stat.h>
@@ -77,20 +76,55 @@ typedef uint_fast64_t random_value;
 #define BASE_62_DIGITS 10 /* 62**10 < UINT_FAST64_MAX */
 #define BASE_62_POWER (62LL * 62 * 62 * 62 * 62 * 62 * 62 * 62 * 62 * 62)
 
+/* Return the result of mixing the entropy from R and S.
+   Assume that R and S are not particularly random,
+   and that the result should look randomish to an untrained eye.  */
+
 static random_value
-random_bits (random_value var, bool use_getrandom)
+mix_random_values (random_value r, random_value s)
+{
+  /* As this code is used only when high-quality randomness is neither
+     available nor necessary, there is no need for fancier polynomials
+     such as those in the Linux kernel's 'random' driver.  */
+  return (2862933555777941757 * r + 3037000493) ^ s;
+}
+
+/* Set *R to a random value.
+   Return true if *R is set to high-quality value taken from getrandom.
+   Otherwise return false, falling back to a low-quality *R that might
+   depend on S.
+
+   This function returns false only when getrandom fails.
+   On GNU systems this should happen only early in the boot process,
+   when the fallback should be good enough for programs using tempname
+   because any attacker likely has root privileges already.  */
+
+static bool
+random_bits (random_value *r, random_value s)
 {
-  random_value r;
   /* Without GRND_NONBLOCK it can be blocked for minutes on some systems.  */
-  if (use_getrandom && __getrandom (&r, sizeof r, GRND_NONBLOCK) == sizeof r)
-    return r;
-#if _LIBC || (defined CLOCK_MONOTONIC && HAVE_CLOCK_GETTIME)
-  /* Add entropy if getrandom did not work.  */
+  if (__getrandom (r, sizeof *r, GRND_NONBLOCK) == sizeof *r)
+    return true;
+
+  /* If getrandom did not work, use ersatz entropy based on low-order
+     clock bits.  On GNU systems getrandom should fail only
+     early in booting, when ersatz should be good enough.
+     Do not use ASLR-based entropy, as that would leak ASLR info into
+     the resulting file name which is typically public.
+
+     Of course we are in a state of sin here.  */
+
+  random_value v = s;
+
+#if _LIBC || (defined CLOCK_REALTIME && HAVE_CLOCK_GETTIME)
   struct __timespec64 tv;
-  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
-  var ^= tv.tv_nsec;
+  __clock_gettime64 (CLOCK_REALTIME, &tv);
+  v = mix_random_values (v, tv.tv_sec);
+  v = mix_random_values (v, tv.tv_nsec);
 #endif
-  return 2862933555777941757 * var + 3037000493;
+
+  *r = mix_random_values (v, clock ());
+  return false;
 }
 
 #if _LIBC
@@ -213,7 +247,7 @@ static const char letters[] =
                         and return a read-write fd.  The file is mode 0600.
    __GT_DIR:            create a directory, which will be mode 0700.
 
-   We use a clever algorithm to get hard-to-predict names. */
+   */
 #ifdef _LIBC
 static
 #endif
@@ -261,25 +295,17 @@ try_tempname_len (char *tmpl, int suffixlen, void *args,
   unsigned int attempts = ATTEMPTS_MIN;
 #endif
 
-  /* A random variable.  The initial value is used only the for fallback path
-     on 'random_bits' on 'getrandom' failure.  Its initial value tries to use
-     some entropy from the ASLR and ignore possible bits from the stack
-     alignment.  */
-  random_value v = ((uintptr_t) &v) / alignof (max_align_t);
+  /* A random variable.  */
+  random_value v = 0;
 
-  /* How many random base-62 digits can currently be extracted from V.  */
+  /* A value derived from the random variable, and how many random
+     base-62 digits can currently be extracted from VDIGBUF.  */
+  random_value vdigbuf;
   int vdigits = 0;
 
-  /* Whether to consume entropy when acquiring random bits.  On the
-     first try it's worth the entropy cost with __GT_NOCREATE, which
-     is inherently insecure and can use the entropy to make it a bit
-     less secure.  On the (rare) second and later attempts it might
-     help against DoS attacks.  */
-  bool use_getrandom = tryfunc == try_nocreate;
-
-  /* Least unfair value for V.  If V is less than this, V can generate
-     BASE_62_DIGITS digits fairly.  Otherwise it might be biased.  */
-  random_value const unfair_min
+  /* Least biased value for V.  If V is less than this, V can generate
+     BASE_62_DIGITS unbiased digits.  Otherwise the digits are biased.  */
+  random_value const biased_min
     = RANDOM_VALUE_MAX - RANDOM_VALUE_MAX % BASE_62_POWER;
 
   len = strlen (tmpl);
@@ -299,18 +325,16 @@ try_tempname_len (char *tmpl, int suffixlen, void *args,
         {
           if (vdigits == 0)
             {
-              do
-                {
-                  v = random_bits (v, use_getrandom);
-                  use_getrandom = true;
-                }
-              while (unfair_min <= v);
+              /* Worry about bias only if the bits are high quality.  */
+              while (random_bits (&v, v) && biased_min <= v)
+                continue;
 
+              vdigbuf = v;
               vdigits = BASE_62_DIGITS;
             }
 
-          XXXXXX[i] = letters[v % 62];
-          v /= 62;
+          XXXXXX[i] = letters[vdigbuf % 62];
+          vdigbuf /= 62;
           vdigits--;
         }
 
-- 
2.37.2


  parent reply	other threads:[~2022-08-22 20:58 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-22 20:58 [PATCH 1/3] Merge _GL_UNUSED C23 " Paul Eggert
2022-08-22 20:58 ` [PATCH 2/3] Merge getopt " Paul Eggert
2022-08-23  7:19   ` Florian Weimer
2022-08-22 20:58 ` Paul Eggert [this message]
2022-08-23  7:19   ` [PATCH 3/3] Merge tempname ASLR etc. " Florian Weimer
2022-08-24  4:15     ` Paul Eggert
2022-08-23  7:16 ` [PATCH 1/3] Merge _GL_UNUSED C23 " Florian Weimer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220822205834.563590-3-eggert@cs.ucla.edu \
    --to=eggert@cs.ucla.edu \
    --cc=libc-alpha@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).