random-util: remove RDRAND usage

zx2c4 · bluca · commit ffa047a03e4c · 2022-03-14T19:47:13.000Z
/dev/urandom is seeded with RDRAND. Calling genuine_random_bytes(...,
..., 0) will use /dev/urandom as a last resort. Hence, we gain nothing
here by having our own RDRAND wrapper, because /dev/urandom already is
based on RDRAND output, even before /dev/urandom has fully initialized.

Furthermore, RDRAND is not actually fast! And on each successive
generation of new x86 CPUs, from both AMD and Intel, it just gets
slower.

This commit simplifies things by just using /dev/urandom in cases where
we before might use RDRAND, since /dev/urandom will always have RDRAND
mixed in as part of it.

And above where I say "/dev/urandom", what I actually mean is
GRND_INSECURE, which is the same thing but won't generate warnings in
dmesg.
diff --git a/NEWS b/NEWS
@@ -95,6 +95,20 @@ CHANGES WITH 251:
           handling, and improving compatibility with home directories intended
           to be portable like the ones managed by systemd-homed.
 
+        * All kernels supported by systemd mix RDRAND (or similar) into the
+          entropy pool at early boot. This means that on those systems, even
+          if /dev/urandom is not yet initialized, it still returns bytes that
+          that are at least as high quality as RDRAND. For that reason, we no
+          longer have reason to invoke RDRAND from systemd itself, which has
+          historically been a source of bugs. Furthermore, kernels ≥5.6 provide
+          the getrandom(GRND_INSECURE) interface for returning random bytes
+          before the entropy pool is initialized without warning into kmsg,
+          which is what we attempt to use if available. By removing systemd's
+          direct usage of RDRAND, x86 systems ≥Broadwell that are running an
+          older kernel may experience kmsg warnings that were not seen with
+          250. For newer kernels, non-x86 systems, or older x86 systems,
+          there should be no visible changes.
+
 CHANGES WITH 250:
 
         * Support for encrypted and authenticated credentials has been added.
diff --git a/docs/ENVIRONMENT.md b/docs/ENVIRONMENT.md
@@ -97,9 +97,6 @@ All tools:
   systems built with libxcrypt and is ignored on systems using glibc's
   original, internal `crypt()` implementation.)
 
-* `$SYSTEMD_RDRAND=0` — if set, the RDRAND instruction will never be used,
-  even if the CPU supports it.
-
 * `$SYSTEMD_SECCOMP=0` — if set, seccomp filters will not be enforced, even if
   support for it is compiled in and available in the kernel.
 
diff --git a/docs/PORTING_TO_NEW_ARCHITECTURES.md b/docs/PORTING_TO_NEW_ARCHITECTURES.md
@@ -53,9 +53,6 @@ architecture.
    support booting into OS trees that have an empty root directory with only
    `/usr/` mounted in.
 
-7. If your architecture has a CPU opcode similar to x86' RDRAND consider adding
-   native support for it to `src/basic/random-util.c`'s `rdrand()` function.
-
-8. If your architecture supports VM virtualization and provides CPU opcodes
+7. If your architecture supports VM virtualization and provides CPU opcodes
    similar to x86' CPUID consider adding native support for detecting VMs this
    way to `src/basic/virt.c`.
diff --git a/docs/RANDOM_SEEDS.md b/docs/RANDOM_SEEDS.md
@@ -144,33 +144,11 @@ acquired.
 ## Keeping `systemd'`s Demand on the Kernel Entropy Pool Minimal
 
 Since most of systemd's own use of random numbers do not require
-cryptographic-grade RNGs, it tries to avoid reading entropy from the kernel
-entropy pool if possible. If it succeeds this has the benefit that there's no
-need to delay the early boot process until entropy is available, and noisy
-kernel log messages about early reading from `/dev/urandom` are avoided
-too. Specifically:
-
-1. When generating [Type 4
-   UUIDs](https://en.wikipedia.org/wiki/Universally_unique_identifier#Version_4_\(random\)),
-   systemd tries to use Intel's and AMD's RDRAND CPU opcode directly, if
-   available. While some doubt the quality and trustworthiness of the entropy
-   provided by these opcodes, they should be good enough for generating UUIDs,
-   if not key material (though, as mentioned, today's big distributions opted
-   to trust it for that too, now, see above — but we are not going to make that
-   decision for you, and for anything key material related will only use the
-   kernel's entropy pool). If RDRAND is not available or doesn't work, it will
-   use synchronous `getrandom()` as fallback, and `/dev/urandom` on old kernels
-   where that system call doesn't exist yet. This means on non-Intel/AMD
-   systems UUID generation will block on kernel entropy initialization.
-
-2. For seeding hash tables, and all the other similar purposes systemd first
-   tries RDRAND, and if that's not available will try to use asynchronous
-   `getrandom()` (if the kernel doesn't support this system call,
-   `/dev/urandom` is used). This may fail too in case the pool is not
-   initialized yet, in which case it will fall back to glibc's internal rand()
-   calls, i.e. weak pseudo-random numbers. This should make sure we use good
-   random bytes if we can, but neither delay boot nor trigger noisy kernel log
-   messages during early boot for these use-cases.
+cryptographic-grade RNGs, it tries to avoid blocking reads to the kernel's RNG,
+opting instead for using `getrandom(GRND_INSECURE)`. After the pool is
+initialized, this is identical to `getrandom(0)`, returning cryptographically
+secure random numbers, but before it's initialized it has the nice effect of
+not blocking system boot.
 
 ## `systemd`'s Support for Filling the Kernel Entropy Pool
 
@@ -280,10 +258,8 @@ early-boot entropy in most cases. Specifically:
    hosting provider if they don't. For VMs used in testing environments,
    `systemd.random_seed=` may be used as an alternative to a virtualized RNG.
 
-3. On Intel/AMD systems systemd's own reliance on the kernel entropy pool is
-   minimal (as RDRAND is used on those for UUID generation). This only works if
-   the CPU has RDRAND of course, which most physical CPUs do (but I hear many
-   virtualized CPUs do not. Pity.)
+3. In general, systemd's own reliance on the kernel entropy pool is minimal
+   (due to the use of `GRND_INSECURE`).
 
 4. In all other cases, `systemd-random-seed.service` will help a bit, but — as
    mentioned — is too late to help with early boot.
diff --git a/src/basic/random-util.c b/src/basic/random-util.c
@@ -35,137 +35,11 @@
 
 static bool srand_called = false;
 
-int rdrand(unsigned long *ret) {
-
-        /* So, you are a "security researcher", and you wonder why we bother with using raw RDRAND here,
-         * instead of sticking to /dev/urandom or getrandom()?
-         *
-         * Here's why: early boot. On Linux, during early boot the random pool that backs /dev/urandom and
-         * getrandom() is generally not initialized yet. It is very common that initialization of the random
-         * pool takes a longer time (up to many minutes), in particular on embedded devices that have no
-         * explicit hardware random generator, as well as in virtualized environments such as major cloud
-         * installations that do not provide virtio-rng or a similar mechanism.
-         *
-         * In such an environment using getrandom() synchronously means we'd block the entire system boot-up
-         * until the pool is initialized, i.e. *very* long. Using getrandom() asynchronously (GRND_NONBLOCK)
-         * would mean acquiring randomness during early boot would simply fail. Using /dev/urandom would mean
-         * generating many kmsg log messages about our use of it before the random pool is properly
-         * initialized. Neither of these outcomes is desirable.
-         *
-         * Thus, for very specific purposes we use RDRAND instead of either of these three options. RDRAND
-         * provides us quickly and relatively reliably with random values, without having to delay boot,
-         * without triggering warning messages in kmsg.
-         *
-         * Note that we use RDRAND only under very specific circumstances, when the requirements on the
-         * quality of the returned entropy permit it. Specifically, here are some cases where we *do* use
-         * RDRAND:
-         *
-         *         • UUID generation: UUIDs are supposed to be universally unique but are not cryptographic
-         *           key material. The quality and trust level of RDRAND should hence be OK: UUIDs should be
-         *           generated in a way that is reliably unique, but they do not require ultimate trust into
-         *           the entropy generator. systemd generates a number of UUIDs during early boot, including
-         *           'invocation IDs' for every unit spawned that identify the specific invocation of the
-         *           service globally, and a number of others. Other alternatives for generating these UUIDs
-         *           have been considered, but don't really work: for example, hashing uuids from a local
-         *           system identifier combined with a counter falls flat because during early boot disk
-         *           storage is not yet available (think: initrd) and thus a system-specific ID cannot be
-         *           stored or retrieved yet.
-         *
-         *         • Hash table seed generation: systemd uses many hash tables internally. Hash tables are
-         *           generally assumed to have O(1) access complexity, but can deteriorate to prohibitive
-         *           O(n) access complexity if an attacker manages to trigger a large number of hash
-         *           collisions. Thus, systemd (as any software employing hash tables should) uses seeded
-         *           hash functions for its hash tables, with a seed generated randomly. The hash tables
-         *           systemd employs watch the fill level closely and reseed if necessary. This allows use of
-         *           a low quality RNG initially, as long as it improves should a hash table be under attack:
-         *           the attacker after all needs to trigger many collisions to exploit it for the purpose
-         *           of DoS, but if doing so improves the seed the attack surface is reduced as the attack
-         *           takes place.
-         *
-         * Some cases where we do NOT use RDRAND are:
-         *
-         *         • Generation of cryptographic key material 🔑
-         *
-         *         • Generation of cryptographic salt values 🧂
-         *
-         * This function returns:
-         *
-         *         -EOPNOTSUPP → RDRAND is not available on this system 😔
-         *         -EAGAIN     → The operation failed this time, but is likely to work if you try again a few
-         *                       times ♻
-         *         -EUCLEAN    → We got some random value, but it looked strange, so we refused using it.
-         *                       This failure might or might not be temporary. 😕
-         */
-
-#if defined(__i386__) || defined(__x86_64__)
-        static int have_rdrand = -1;
-        unsigned long v;
-        uint8_t success;
-
-        if (have_rdrand < 0) {
-                uint32_t eax, ebx, ecx, edx;
-
-                /* Check if RDRAND is supported by the CPU */
-                if (__get_cpuid(1, &eax, &ebx, &ecx, &edx) == 0) {
-                        have_rdrand = false;
-                        return -EOPNOTSUPP;
-                }
-
-/* Compat with old gcc where bit_RDRND didn't exist yet */
-#ifndef bit_RDRND
-#define bit_RDRND (1U << 30)
-#endif
-
-                have_rdrand = !!(ecx & bit_RDRND);
-
-                if (have_rdrand > 0) {
-                        /* Allow disabling use of RDRAND with SYSTEMD_RDRAND=0
-                           If it is unset getenv_bool_secure will return a negative value. */
-                        if (getenv_bool_secure("SYSTEMD_RDRAND") == 0) {
-                                have_rdrand = false;
-                                return -EOPNOTSUPP;
-                        }
-                }
-        }
-
-        if (have_rdrand == 0)
-                return -EOPNOTSUPP;
-
-        asm volatile("rdrand %0;"
-                     "setc %1"
-                     : "=r" (v),
-                       "=qm" (success));
-        msan_unpoison(&success, sizeof(success));
-        if (!success)
-                return -EAGAIN;
-
-        /* Apparently on some AMD CPUs RDRAND will sometimes (after a suspend/resume cycle?) report success
-         * via the carry flag but nonetheless return the same fixed value -1 in all cases. This appears to be
-         * a bad bug in the CPU or firmware. Let's deal with that and work-around this by explicitly checking
-         * for this special value (and also 0, just to be sure) and filtering it out. This is a work-around
-         * only however and something AMD really should fix properly. The Linux kernel should probably work
-         * around this issue by turning off RDRAND altogether on those CPUs. See:
-         * https://github.com/systemd/systemd/issues/11810 */
-        if (v == 0 || v == ULONG_MAX)
-                return log_debug_errno(SYNTHETIC_ERRNO(EUCLEAN),
-                                       "RDRAND returned suspicious value %lx, assuming bad hardware RNG, not using value.", v);
-
-        *ret = v;
-        return 0;
-#else
-        return -EOPNOTSUPP;
-#endif
-}
-
 int genuine_random_bytes(void *p, size_t n, RandomFlags flags) {
         static int have_syscall = -1;
         _cleanup_close_ int fd = -1;
 
-        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
+        /* Gathers some high-quality randomness from the kernel. This call won't block, unless the RANDOM_BLOCK
          * 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. */
@@ -212,34 +86,6 @@ int genuine_random_bytes(void *p, size_t n, RandomFlags flags) {
                 }
         }
 
-        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) {
-                                /* OK, this didn't work, let's go with /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! */
-                }
-        }
-
         fd = open("/dev/urandom", O_RDONLY|O_CLOEXEC|O_NOCTTY);
         if (fd < 0)
                 return errno == ENOENT ? -ENOSYS : -errno;
@@ -257,8 +103,6 @@ void initialize_srand(void) {
 #if HAVE_SYS_AUXV_H
         const void *auxv;
 #endif
-        unsigned long k;
-
         if (srand_called)
                 return;
 
@@ -283,9 +127,6 @@ void initialize_srand(void) {
         x ^= (unsigned) now(CLOCK_REALTIME);
         x ^= (unsigned) gettid();
 
-        if (rdrand(&k) >= 0)
-                x ^= (unsigned) k;
-
         srand(x);
         srand_called = true;
 
@@ -339,8 +180,8 @@ void random_bytes(void *p, size_t n) {
          *
          * What this function will do:
          *
-         *         • This function will preferably use the CPU's RDRAND operation, if it is available, in
-         *           order to return "mid-quality" random values cheaply.
+         *         • Use getrandom(GRND_INSECURE) or /dev/urandom, to return high-quality random values if
+         *           they are cheaply available, or less high-quality random values if they are not.
          *
          *         • This function will return pseudo-random data, generated via libc rand() if nothing
          *           better is available.
@@ -363,7 +204,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_ALLOW_RDRAND) >= 0)
+        if (genuine_random_bytes(p, n, 0) >= 0)
                 return;
 
         /* If for some reason some user made /dev/urandom unavailable to us, or the kernel has no entropy, use a PRNG instead. */
diff --git a/src/basic/random-util.h b/src/basic/random-util.h
@@ -7,7 +7,6 @@
 
 typedef enum RandomFlags {
         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 */
@@ -28,8 +27,6 @@ static inline uint32_t random_u32(void) {
         return u;
 }
 
-int rdrand(unsigned long *ret);
-
 /* Some limits on the pool sizes when we deal with the kernel random pool */
 #define RANDOM_POOL_SIZE_MIN 512U
 #define RANDOM_POOL_SIZE_MAX (10U*1024U*1024U)
diff --git a/src/libsystemd/sd-id128/sd-id128.c b/src/libsystemd/sd-id128/sd-id128.c
@@ -276,9 +276,7 @@ _public_ int sd_id128_randomize(sd_id128_t *ret) {
 
         assert_return(ret, -EINVAL);
 
-        /* We allow usage if x86-64 RDRAND here. It might not be trusted enough for keeping secrets, but it should be
-         * fine for UUIDS. */
-        r = genuine_random_bytes(&t, sizeof t, RANDOM_ALLOW_RDRAND);
+        r = genuine_random_bytes(&t, sizeof(t), 0);
         if (r < 0)
                 return r;
 
diff --git a/src/test/test-random-util.c b/src/test/test-random-util.c
@@ -26,7 +26,6 @@ static void test_genuine_random_bytes_one(RandomFlags flags) {
 TEST(genuine_random_bytes) {
         test_genuine_random_bytes_one(0);
         test_genuine_random_bytes_one(RANDOM_BLOCK);
-        test_genuine_random_bytes_one(RANDOM_ALLOW_RDRAND);
 }
 
 TEST(pseudo_random_bytes) {
@@ -41,22 +40,6 @@ TEST(pseudo_random_bytes) {
         }
 }
 
-TEST(rdrand) {
-        int r;
-
-        for (unsigned i = 0; i < 10; i++) {
-                unsigned long x = 0;
-
-                r = rdrand(&x);
-                if (r < 0) {
-                        log_error_errno(r, "RDRAND failed: %m");
-                        return;
-                }
-
-                printf("%lx\n", x);
-        }
-}
-
 #define TOTAL 100000
 
 static void test_random_u64_range_one(unsigned mod) {
diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c
@@ -622,10 +622,9 @@ static int link_generate_new_hw_addr(Link *link, struct hw_addr_data *ret) {
 
         if (link->config->mac_address_policy == MAC_ADDRESS_POLICY_RANDOM)
                 /* We require genuine randomness here, since we want to make sure we won't collide with other
-                 * systems booting up at the very same time. We do allow RDRAND however, since this is not
-                 * cryptographic key material. */
+                 * systems booting up at the very same time. */
                 for (;;) {
-                        r = genuine_random_bytes(p, len, RANDOM_ALLOW_RDRAND);
+                        r = genuine_random_bytes(p, len, 0);
                         if (r < 0)
                                 return log_link_warning_errno(link, r, "Failed to acquire random data to generate MAC address: %m");
 
