From 6a06b1a5d9d6a05404ea6ace739d39e6256968c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 25 Jun 2017 17:32:53 -0400 Subject: [PATCH 1/3] basic/random-util: use most of the pseudorandom bytes from rand() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The only implementation that we care about — glibc — provides us with 31 bits of entropy. Let's use 24 bits of that, instead of throwing all but 8 away. --- src/basic/random-util.c | 41 +++++++++++++++++++++++++++++++++-------- src/basic/random-util.h | 1 + 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/basic/random-util.c b/src/basic/random-util.c index b216be579d..b2ca2cc8e3 100644 --- a/src/basic/random-util.c +++ b/src/basic/random-util.c @@ -121,19 +121,44 @@ void initialize_srand(void) { srand_called = true; } -void random_bytes(void *p, size_t n) { +/* INT_MAX gives us only 31 bits, so use 24 out of that. */ +#if RAND_MAX >= INT_MAX +# define RAND_STEP 3 +#else +/* SHORT_INT_MAX or lower gives at most 15 bits, we just just 8 out of that. */ +# define RAND_STEP 1 +#endif + +void pseudorandom_bytes(void *p, size_t n) { uint8_t *q; + + initialize_srand(); + + for (q = p; q < (uint8_t*) p + n; q += RAND_STEP) { + unsigned rr; + + rr = (unsigned) rand(); + +#if RAND_STEP >= 3 + if (q - (uint8_t*) p + 2 < n) + q[2] = rr >> 16; +#endif +#if RAND_STEP >= 2 + if (q - (uint8_t*) p + 1 < n) + q[1] = rr >> 8; +#endif + q[0] = rr; + } +} + +void random_bytes(void *p, size_t n) { int r; r = dev_urandom(p, n); if (r >= 0) return; - /* If some idiot made /dev/urandom unavailable to us, he'll - * get a PRNG instead. */ - - initialize_srand(); - - for (q = p; q < (uint8_t*) p + n; q ++) - *q = rand(); + /* If some idiot made /dev/urandom unavailable to us, or the + * kernel has no entropy, use a PRNG instead. */ + return pseudorandom_bytes(p, n); } diff --git a/src/basic/random-util.h b/src/basic/random-util.h index 3cee4c5014..294ef4d4e5 100644 --- a/src/basic/random-util.h +++ b/src/basic/random-util.h @@ -23,6 +23,7 @@ #include int dev_urandom(void *p, size_t n); +void pseudorandom_bytes(void *p, size_t n); void random_bytes(void *p, size_t n); void initialize_srand(void); From f0d09059bd4a195840f7c79943be942b240d5500 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 25 Jun 2017 17:09:05 -0400 Subject: [PATCH 2/3] basic/random-util: do not fall back to /dev/urandom if getrandom() returns short MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During early boot, we'd call getrandom(), and immediately fall back to reading from /dev/urandom unless we got the full requested number of bytes. Those two sources are the same, so the most likely result is /dev/urandom producing some pseudorandom numbers for us, complaining widely on the way. Let's change our behaviour to be more conservative: - if the numbers are only used to initialize a hash table, a short read is OK, we don't really care if we get the first part of the seed truly random and then some pseudorandom bytes. So just do that and return "success". - if getrandom() returns -EAGAIN, fall back to rand() instead of querying /dev/urandom again. The idea with those two changes is to avoid generating a warning about reading from an /dev/urandom when the kernel doesn't have enough entropy. - only in the cases where we really need to make the best effort possible (sd_id128_randomize and firstboot password hashing), fall back to /dev/urandom. When calling getrandom(), drop the checks whether the argument fits in an int — getrandom() should do that for us already, and we call it with small arguments only anyway. Note that this does not really change the (relatively high) number of random bytes we request from the kernel. On my laptop, during boot, PID 1 and all other processes using this code through libsystemd request: 74780 bytes with high_quality_required == false 464 bytes with high_quality_required == true and it does not eliminate reads from /dev/urandom completely. If the kernel was short on entropy and getrandom() would fail, we would fall back to /dev/urandom for those 464 bytes. When falling back to /dev/urandom, don't lose the short read we already got, and just read the remaining bytes. If getrandom() syscall is not available, we fall back to /dev/urandom same as before. Fixes #4167 (possibly partially, let's see). --- src/basic/random-util.c | 75 ++++++++++++++++-------------- src/basic/random-util.h | 3 +- src/firstboot/firstboot.c | 2 +- src/libsystemd/sd-id128/sd-id128.c | 2 +- 4 files changed, 45 insertions(+), 37 deletions(-) diff --git a/src/basic/random-util.c b/src/basic/random-util.c index b2ca2cc8e3..490107ef41 100644 --- a/src/basic/random-util.c +++ b/src/basic/random-util.c @@ -42,53 +42,59 @@ #include "random-util.h" #include "time-util.h" -int dev_urandom(void *p, size_t n) { +int acquire_random_bytes(void *p, size_t n, bool high_quality_required) { static int have_syscall = -1; _cleanup_close_ int fd = -1; int r; + unsigned already_done = 0; - /* Gathers some randomness from the kernel. This call will - * never block, and will always return some data from the - * kernel, regardless if the random pool is fully initialized - * or not. It thus makes no guarantee for the quality of the - * returned entropy, but is good enough for our usual usecases - * of seeding the hash functions for hashtable */ + /* Gathers some randomness from the kernel. This call will never block. If + * high_quality_required, it will always return some data from the kernel, + * regardless of whether the random pool is fully initialized or not. + * Otherwise, it will return success if at least some random bytes were + * successfully acquired, and an error if the kernel has no entropy whatsover + * for us. */ - /* Use the getrandom() syscall unless we know we don't have - * it, or when the requested size is too large for it. */ - if (have_syscall != 0 || (size_t) (int) n != n) { + /* Use the getrandom() syscall unless we know we don't have it. */ + if (have_syscall != 0) { r = getrandom(p, n, GRND_NONBLOCK); - if (r == (int) n) { + if (r > 0) { have_syscall = true; - return 0; - } + if (r == n) + return 0; + if (!high_quality_required) { + /* Fill in the remaing bytes using pseudorandom values */ + pseudorandom_bytes((uint8_t*) p + r, n - r); + return 0; + } - if (r < 0) { - if (errno == ENOSYS) - /* we lack the syscall, continue with - * reading from /dev/urandom */ - have_syscall = false; - else if (errno == EAGAIN) - /* not enough entropy for now. Let's - * remember to use the syscall the - * next time, again, but also read - * from /dev/urandom for now, which - * doesn't care about the current - * amount of entropy. */ - have_syscall = true; - else - return -errno; + already_done = r; + } else if (errno == ENOSYS) + /* We lack the syscall, continue with reading from /dev/urandom. */ + have_syscall = false; + else if (errno == EAGAIN) { + /* The kernel has no entropy whatsoever. Let's remember to + * use the syscall the next time again though. + * + * If high_quality_required is false, return an error so that + * random_bytes() can produce some pseudorandom + * bytes. 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 (!high_quality_required) + return -ENODATA; } else - /* too short read? */ - return -ENODATA; + return -errno; } fd = open("/dev/urandom", O_RDONLY|O_CLOEXEC|O_NOCTTY); if (fd < 0) return errno == ENOENT ? -ENOSYS : -errno; - return loop_read_exact(fd, p, n, true); + return loop_read_exact(fd, (uint8_t*) p + already_done, n - already_done, true); } void initialize_srand(void) { @@ -102,8 +108,9 @@ void initialize_srand(void) { return; #ifdef HAVE_SYS_AUXV_H - /* The kernel provides us with 16 bytes of entropy in auxv, so let's try to make use of that to seed the - * pseudo-random generator. It's better than nothing... */ + /* The kernel provides us with 16 bytes of entropy in auxv, so let's + * try to make use of that to seed the pseudo-random generator. It's + * better than nothing... */ auxv = (void*) getauxval(AT_RANDOM); if (auxv) { @@ -154,7 +161,7 @@ void pseudorandom_bytes(void *p, size_t n) { void random_bytes(void *p, size_t n) { int r; - r = dev_urandom(p, n); + r = acquire_random_bytes(p, n, false); if (r >= 0) return; diff --git a/src/basic/random-util.h b/src/basic/random-util.h index 294ef4d4e5..804e225fc1 100644 --- a/src/basic/random-util.h +++ b/src/basic/random-util.h @@ -19,10 +19,11 @@ along with systemd; If not, see . ***/ +#include #include #include -int dev_urandom(void *p, size_t n); +int acquire_random_bytes(void *p, size_t n, bool high_quality_required); void pseudorandom_bytes(void *p, size_t n); void random_bytes(void *p, size_t n); void initialize_srand(void); diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c index bc16290c72..b3578d3e16 100644 --- a/src/firstboot/firstboot.c +++ b/src/firstboot/firstboot.c @@ -572,7 +572,7 @@ static int process_root_password(void) { if (!arg_root_password) return 0; - r = dev_urandom(raw, 16); + r = acquire_random_bytes(raw, 16, true); if (r < 0) return log_error_errno(r, "Failed to get salt: %m"); diff --git a/src/libsystemd/sd-id128/sd-id128.c b/src/libsystemd/sd-id128/sd-id128.c index cc89f2de2e..b1f6c5305f 100644 --- a/src/libsystemd/sd-id128/sd-id128.c +++ b/src/libsystemd/sd-id128/sd-id128.c @@ -289,7 +289,7 @@ _public_ int sd_id128_randomize(sd_id128_t *ret) { assert_return(ret, -EINVAL); - r = dev_urandom(&t, sizeof(t)); + r = acquire_random_bytes(&t, sizeof t, true); if (r < 0) return r; From 2416f73be12dbac0d121f23f654f7019ae3e4ca8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 25 Jun 2017 18:01:02 -0400 Subject: [PATCH 3/3] tests: add test-random-util In case you're wondering: 16 aligns in a nice pyramid. --- .gitignore | 1 + Makefile.am | 7 ++++ src/test/meson.build | 4 +++ src/test/test-random-util.c | 65 +++++++++++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+) create mode 100644 src/test/test-random-util.c diff --git a/.gitignore b/.gitignore index 60eda2b8ce..c8c59eba56 100644 --- a/.gitignore +++ b/.gitignore @@ -267,6 +267,7 @@ /test-process-util /test-pty /test-qcow2 +/test-random-util /test-ratelimit /test-replace-var /test-resolve diff --git a/Makefile.am b/Makefile.am index 4838df69f9..56f159b84e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1624,6 +1624,7 @@ tests += \ test-conf-parser \ test-capability \ test-async \ + test-random-util \ test-ratelimit \ test-condition \ test-uid-range \ @@ -1945,6 +1946,12 @@ test_fstab_util_SOURCES = \ test_fstab_util_LDADD = \ libsystemd-shared.la +test_random_util_SOURCES = \ + src/test/test-random-util.c + +test_random_util_LDADD = \ + libsystemd-shared.la + test_ratelimit_SOURCES = \ src/test/test-ratelimit.c diff --git a/src/test/meson.build b/src/test/meson.build index 892d2929ee..7b493a4d05 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -161,6 +161,10 @@ tests += [ [], []], + [['src/test/test-random-util.c'], + [], + []], + [['src/test/test-ratelimit.c'], [], []], diff --git a/src/test/test-random-util.c b/src/test/test-random-util.c new file mode 100644 index 0000000000..50f4da270d --- /dev/null +++ b/src/test/test-random-util.c @@ -0,0 +1,65 @@ +/*** + This file is part of systemd. + + Copyright 2017 Zbigniew Jędrzejewski-Szmek + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see . +***/ + +#include "hexdecoct.h" +#include "random-util.h" +#include "log.h" + +static void test_acquire_random_bytes(bool high_quality_required) { + uint8_t buf[16] = {}; + unsigned i; + + log_info("/* %s */", __func__); + + for (i = 1; i < sizeof buf; i++) { + assert_se(acquire_random_bytes(buf, i, high_quality_required) == 0); + if (i + 1 < sizeof buf) + assert_se(buf[i] == 0); + + hexdump(stdout, buf, i); + } +} + +static void test_pseudorandom_bytes(void) { + uint8_t buf[16] = {}; + unsigned i; + + log_info("/* %s */", __func__); + + for (i = 1; i < sizeof buf; i++) { + pseudorandom_bytes(buf, i); + if (i + 1 < sizeof buf) + assert_se(buf[i] == 0); + + hexdump(stdout, buf, i); + } +} + +int main(int argc, char **argv) { + log_set_max_level(LOG_DEBUG); + log_parse_environment(); + log_open(); + + test_acquire_random_bytes(false); + test_acquire_random_bytes(true); + + test_pseudorandom_bytes(); + + return 0; +}