]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
random-util: unify RANDOM_ALLOW_INSECURE and !RANDOM_BLOCK and simplify
authorJason A. Donenfeld <Jason@zx2c4.com>
Mon, 7 Mar 2022 04:36:19 +0000 (21:36 -0700)
committerLennart Poettering <lennart@poettering.net>
Wed, 9 Mar 2022 13:25:15 +0000 (14:25 +0100)
RANDOM_BLOCK has existed for a long time, but RANDOM_ALLOW_INSECURE was
added more recently, leading to an awkward relationship between the two.
It turns out that only one, RANDOM_BLOCK, is needed.

RANDOM_BLOCK means return cryptographically secure numbers no matter
what. If it's not set, it means try to do that, but if it fails, fall
back to using unseeded randomness.

This part of falling back to unseeded randomness is the intent of
GRND_INSECURE, which is what RANDOM_ALLOW_INSECURE previously aliased.
Rather than having an additional flag for that, it makes more sense to
just use it whenever RANDOM_BLOCK is not set. This saves us the overhead
of having to open up /dev/urandom.

Additionally, when getrandom returns too little data, but not zero data,
we currently fall back to using /dev/urandom if RANDOM_BLOCK is not set.
This doesn't quite make sense, because if getrandom returned seeded data
once, then it will forever after return the same thing as whatever
/dev/urandom does. So in that case, we should just loop again.

Since there's never really a time where /dev/urandom is able to return
some easily but more with difficulty, we can also get rid of
RANDOM_EXTEND_WITH_PSEUDO. Once the RNG is initialized, bytes
should just flow normally.

This also makes RANDOM_MAY_FAIL obsolete, because the only case this ran
was where we'd fall back to /dev/urandom on old kernels and return
GRND_INSECURE bytes on new kernels. So also get rid of that flag.

Finally, since we're always able to use GRND_INSECURE on newer kernels,
and we only fall back to /dev/urandom on older kernels, also only fall
back to using RDRAND on those older kernels. There, the only reason to
have RDRAND is to avoid a kmsg entry about unseeded randomness.

The result of this commit is that we now cascade like this:

  - Use getrandom(0) if RANDOM_BLOCK.
  - Use getrandom(GRND_INSECURE) if !RANDOM_BLOCK.
  - Use /dev/urandom if !RANDOM_BLOCK and no GRND_INSECURE support.
  - Use /dev/urandom if no getrandom() support.
  - Use RDRAND if we would use /dev/urandom for any of the above reasons
    and RANDOM_ALLOW_RDRAND is set.

src/basic/random-util.c
src/basic/random-util.h
src/test/test-random-util.c

index e117330857c3cb6a0820d200c616f2861cc4ac99..227b82f790384fd0bd1f2e34a6f281a11ff91cdc 100644 (file)
@@ -160,93 +160,35 @@ int rdrand(unsigned long *ret) {
 int genuine_random_bytes(void *p, size_t n, RandomFlags flags) {
         static int have_syscall = -1;
         _cleanup_close_ int fd = -1;
-        bool got_some = false;
+
+        if (FLAGS_SET(flags, RANDOM_BLOCK | RANDOM_ALLOW_RDRAND))
+                return -EINVAL;
 
         /* Gathers some high-quality randomness from the kernel (or potentially mid-quality randomness from
          * the CPU if the RANDOM_ALLOW_RDRAND flag is set). This call won't block, unless the RANDOM_BLOCK
-         * flag is set. If RANDOM_MAY_FAIL is set, an error is returned if the random pool is not
-         * initialized. Otherwise it will always return some data from the kernel, regardless of whether the
-         * random pool is fully initialized or not. If RANDOM_EXTEND_WITH_PSEUDO is set, and some but not
-         * enough better quality randomness could be acquired, the rest is filled up with low quality
-         * randomness.
-         *
-         * Of course, when creating cryptographic key material you really shouldn't use RANDOM_ALLOW_DRDRAND
-         * or even RANDOM_EXTEND_WITH_PSEUDO.
-         *
-         * When generating UUIDs it's fine to use RANDOM_ALLOW_RDRAND but not OK to use
-         * RANDOM_EXTEND_WITH_PSEUDO. In fact RANDOM_EXTEND_WITH_PSEUDO is only really fine when invoked via
-         * an "all bets are off" wrapper, such as random_bytes(), see below. */
+         * flag is set. If it doesn't block, it will still always return some data from the kernel, regardless
+         * of whether the random pool is fully initialized or not. When creating cryptographic key material you
+         * should always use RANDOM_BLOCK. */
 
         if (n == 0)
                 return 0;
 
-        if (FLAGS_SET(flags, RANDOM_ALLOW_RDRAND))
-                /* Try x86-64' RDRAND intrinsic if we have it. We only use it if high quality randomness is
-                 * not required, as we don't trust it (who does?). Note that we only do a single iteration of
-                 * RDRAND here, even though the Intel docs suggest calling this in a tight loop of 10
-                 * invocations or so. That's because we don't really care about the quality here. We
-                 * generally prefer using RDRAND if the caller allows us to, since this way we won't upset
-                 * the kernel's random subsystem by accessing it before the pool is initialized (after all it
-                 * will kmsg log about every attempt to do so). */
-                for (;;) {
-                        unsigned long u;
-                        size_t m;
-
-                        if (rdrand(&u) < 0) {
-                                if (got_some && FLAGS_SET(flags, RANDOM_EXTEND_WITH_PSEUDO)) {
-                                        /* Fill in the remaining bytes using pseudo-random values */
-                                        pseudo_random_bytes(p, n);
-                                        return 0;
-                                }
-
-                                /* OK, this didn't work, let's go to getrandom() + /dev/urandom instead */
-                                break;
-                        }
-
-                        m = MIN(sizeof(u), n);
-                        memcpy(p, &u, m);
-
-                        p = (uint8_t*) p + m;
-                        n -= m;
-
-                        if (n == 0)
-                                return 0; /* Yay, success! */
-
-                        got_some = true;
-                }
-
         /* Use the getrandom() syscall unless we know we don't have it. */
         if (have_syscall != 0 && !HAS_FEATURE_MEMORY_SANITIZER) {
-
                 for (;;) {
-                        ssize_t l;
-                        l = getrandom(p, n,
-                                      (FLAGS_SET(flags, RANDOM_BLOCK) ? 0 : GRND_NONBLOCK) |
-                                      (FLAGS_SET(flags, RANDOM_ALLOW_INSECURE) ? GRND_INSECURE : 0));
+                        ssize_t l = getrandom(p, n, FLAGS_SET(flags, RANDOM_BLOCK) ? 0 : GRND_INSECURE);
+
                         if (l > 0) {
                                 have_syscall = true;
 
                                 if ((size_t) l == n)
                                         return 0; /* Yay, success! */
 
+                                /* We didn't get enough data, so try again */
                                 assert((size_t) l < n);
                                 p = (uint8_t*) p + l;
                                 n -= l;
-
-                                if (FLAGS_SET(flags, RANDOM_EXTEND_WITH_PSEUDO)) {
-                                        /* Fill in the remaining bytes using pseudo-random values */
-                                        pseudo_random_bytes(p, n);
-                                        return 0;
-                                }
-
-                                got_some = true;
-
-                                /* Hmm, we didn't get enough good data but the caller insists on good data? Then try again */
-                                if (FLAGS_SET(flags, RANDOM_BLOCK))
-                                        continue;
-
-                                /* Fill in the rest with /dev/urandom */
-                                break;
+                                continue;
 
                         } else if (l == 0) {
                                 have_syscall = true;
@@ -257,41 +199,44 @@ int genuine_random_bytes(void *p, size_t n, RandomFlags flags) {
                                 have_syscall = false;
                                 break;
 
-                        } else if (errno == EAGAIN) {
-                                /* The kernel has no entropy whatsoever. Let's remember to use the syscall
-                                 * the next time again though.
-                                 *
-                                 * If RANDOM_MAY_FAIL is set, return an error so that random_bytes() can
-                                 * produce some pseudo-random bytes instead. Otherwise, fall back to
-                                 * /dev/urandom, which we know is empty, but the kernel will produce some
-                                 * bytes for us on a best-effort basis. */
-                                have_syscall = true;
+                        } else if (errno == EINVAL) {
+                                /* If we previously passed GRND_INSECURE, and this flag isn't known, then
+                                 * we're likely running an old kernel which has getrandom() but not
+                                 * GRND_INSECURE. In this case, fall back to /dev/urandom. */
+                                if (!FLAGS_SET(flags, RANDOM_BLOCK))
+                                        break;
 
-                                if (got_some && FLAGS_SET(flags, RANDOM_EXTEND_WITH_PSEUDO)) {
-                                        /* Fill in the remaining bytes using pseudorandom values */
-                                        pseudo_random_bytes(p, n);
-                                        return 0;
-                                }
+                                return -errno;
+                        } else
+                                return -errno;
+                }
+        }
 
-                                if (FLAGS_SET(flags, RANDOM_MAY_FAIL))
-                                        return -ENODATA;
+        if (FLAGS_SET(flags, RANDOM_ALLOW_RDRAND)) {
+                /* Try x86-64' RDRAND intrinsic if we have it. We only use it if high quality randomness is
+                 * not required, as we don't trust it (who does?). Note that we only do a single iteration of
+                 * RDRAND here, even though the Intel docs suggest calling this in a tight loop of 10
+                 * invocations or so. That's because we don't really care about the quality here. We
+                 * generally prefer using RDRAND if the caller allows us to, since this way we won't upset
+                 * the kernel's random subsystem by accessing it before the pool is initialized (after all it
+                 * will kmsg log about every attempt to do so). */
+                for (;;) {
+                        unsigned long u;
+                        size_t m;
 
-                                /* Use /dev/urandom instead */
+                        if (rdrand(&u) < 0) {
+                                /* OK, this didn't work, let's go with /dev/urandom instead */
                                 break;
+                        }
 
-                        } else if (errno == EINVAL) {
-
-                                /* Most likely: unknown flag. We know that GRND_INSECURE might cause this,
-                                 * hence try without. */
+                        m = MIN(sizeof(u), n);
+                        memcpy(p, &u, m);
 
-                                if (FLAGS_SET(flags, RANDOM_ALLOW_INSECURE)) {
-                                        flags = flags &~ RANDOM_ALLOW_INSECURE;
-                                        continue;
-                                }
+                        p = (uint8_t*) p + m;
+                        n -= m;
 
-                                return -errno;
-                        } else
-                                return -errno;
+                        if (n == 0)
+                                return 0; /* Yay, success! */
                 }
         }
 
@@ -421,7 +366,7 @@ void random_bytes(void *p, size_t n) {
          * This function is hence not useful for generating UUIDs or cryptographic key material.
          */
 
-        if (genuine_random_bytes(p, n, RANDOM_EXTEND_WITH_PSEUDO|RANDOM_MAY_FAIL|RANDOM_ALLOW_RDRAND|RANDOM_ALLOW_INSECURE) >= 0)
+        if (genuine_random_bytes(p, n, RANDOM_ALLOW_RDRAND) >= 0)
                 return;
 
         /* If for some reason some user made /dev/urandom unavailable to us, or the kernel has no entropy, use a PRNG instead. */
index e6528ddc7fe713cde424d5de48e949c8bea0e7a6..99f6c73914c19129343c6faf209e838e7cb09530 100644 (file)
@@ -6,11 +6,8 @@
 #include <stdint.h>
 
 typedef enum RandomFlags {
-        RANDOM_EXTEND_WITH_PSEUDO = 1 << 0, /* If we can't get enough genuine randomness, but some, fill up the rest with pseudo-randomness */
-        RANDOM_BLOCK              = 1 << 1, /* Rather block than return crap randomness (only if the kernel supports that) */
-        RANDOM_MAY_FAIL           = 1 << 2, /* If we can't get any randomness at all, return early with -ENODATA */
-        RANDOM_ALLOW_RDRAND       = 1 << 3, /* Allow usage of the CPU RNG */
-        RANDOM_ALLOW_INSECURE     = 1 << 4, /* Allow usage of GRND_INSECURE flag to kernel's getrandom() API */
+        RANDOM_BLOCK              = 1 << 0, /* Rather block than return crap randomness (only if the kernel supports that) */
+        RANDOM_ALLOW_RDRAND       = 1 << 1, /* Allow usage of the CPU RNG */
 } RandomFlags;
 
 int genuine_random_bytes(void *p, size_t n, RandomFlags flags); /* returns "genuine" randomness, optionally filled up with pseudo random, if not enough is available */
index 2b09a4513a7d418736a6b495fd857937978d594d..3426d606f4184ab94d9095e12a850b7d7c0c75e1 100644 (file)
@@ -24,11 +24,9 @@ static void test_genuine_random_bytes_one(RandomFlags flags) {
 }
 
 TEST(genuine_random_bytes) {
-        test_genuine_random_bytes_one(RANDOM_EXTEND_WITH_PSEUDO);
         test_genuine_random_bytes_one(0);
         test_genuine_random_bytes_one(RANDOM_BLOCK);
         test_genuine_random_bytes_one(RANDOM_ALLOW_RDRAND);
-        test_genuine_random_bytes_one(RANDOM_ALLOW_INSECURE);
 }
 
 TEST(pseudo_random_bytes) {