update TODO

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.

.packit.yml

@@ -13,6 +13,7 @@ downstream_package_name: systemd
 # `git describe` returns in systemd's case 'v245-xxx' which breaks RPM version
 # detection (that expects 245-xxxx'). Let's tweak the version string accordingly
 upstream_tag_template: "v{version}"
+srpm_build_deps: []
 
 actions:
   post-upstream-clone:

TODO

@@ -81,6 +81,9 @@ Features:
 * doc: prep a document explaining resolved's internal objects, i.e. Query
   vs. Question vs. Transaction vs. Stream and so on.
 
+* doc: prep a document explaining PID 1's internal logic, i.e. transactions,
+  jobs, units
+
 * userbdctl: show user types via colors, also display UID range info in some way
 
 * bootspec: remove tries counter from boot entry ids

src/basic/random-util.c

@@ -160,27 +160,59 @@ 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
+         * flag is set. If it doesn't block, it will still always return some data from the kernel, regardless
-         * initialized. Otherwise it will always return some data from the kernel, regardless of whether the
+         * of whether the random pool is fully initialized or not. When creating cryptographic key material you
-         * random pool is fully initialized or not. If RANDOM_EXTEND_WITH_PSEUDO is set, and some but not
+         * should always use RANDOM_BLOCK. */
-         * 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. */
 
         if (n == 0)
                 return 0;
 
-        if (FLAGS_SET(flags, RANDOM_ALLOW_RDRAND))
+        /* Use the getrandom() syscall unless we know we don't have it. */
+        if (have_syscall != 0 && !HAS_FEATURE_MEMORY_SANITIZER) {
+                for (;;) {
+                        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;
+                                continue;
+
+                        } else if (l == 0) {
+                                have_syscall = true;
+                                return -EIO;
+
+                        } else if (ERRNO_IS_NOT_SUPPORTED(errno)) {
+                                /* We lack the syscall, continue with reading from /dev/urandom. */
+                                have_syscall = false;
+                                break;
+
+                        } 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;
+
+                                return -errno;
+                        } else
+                                return -errno;
+                }
+        }
+
+        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
@@ -193,13 +225,7 @@ int genuine_random_bytes(void *p, size_t n, RandomFlags flags) {
                         size_t m;
 
                         if (rdrand(&u) < 0) {
-                                if (got_some && FLAGS_SET(flags, RANDOM_EXTEND_WITH_PSEUDO)) {
+                                /* OK, this didn't work, let's go with /dev/urandom instead */
-                                        /* 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;
                         }
 
@@ -211,87 +237,6 @@ int genuine_random_bytes(void *p, size_t n, RandomFlags flags) {
 
                         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));
-                        if (l > 0) {
-                                have_syscall = true;
-
-                                if ((size_t) l == n)
-                                        return 0; /* Yay, success! */
-
-                                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;
-
-                        } else if (l == 0) {
-                                have_syscall = true;
-                                return -EIO;
-
-                        } else if (ERRNO_IS_NOT_SUPPORTED(errno)) {
-                                /* We lack the syscall, continue with reading from /dev/urandom. */
-                                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;
-
-                                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;
-                                }
-
-                                if (FLAGS_SET(flags, RANDOM_MAY_FAIL))
-                                        return -ENODATA;
-
-                                /* Use /dev/urandom instead */
-                                break;
-
-                        } else if (errno == EINVAL) {
-
-                                /* Most likely: unknown flag. We know that GRND_INSECURE might cause this,
-                                 * hence try without. */
-
-                                if (FLAGS_SET(flags, RANDOM_ALLOW_INSECURE)) {
-                                        flags = flags &~ RANDOM_ALLOW_INSECURE;
-                                        continue;
-                                }
-
-                                return -errno;
-                        } else
-                                return -errno;
                 }
         }
 
@@ -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. */

src/basic/random-util.h

@@ -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 << 0, /* Rather block than return crap randomness (only if the kernel supports that) */
-        RANDOM_BLOCK              = 1 << 1, /* Rather block than return crap randomness (only if the kernel supports that) */
+        RANDOM_ALLOW_RDRAND       = 1 << 1, /* Allow usage of the CPU RNG */
-        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 */
 } 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 */

src/test/test-random-util.c

@@ -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) {