Re: [PATCH RFC v2] random: optionally block in getrandom(2) when the CRNG is uninitialized

[Linus Torvalds]

[ Added Lennart, who was active in the other thread ]

On Sat, Sep 14, 2019 at 10:22 PM Theodore Y. Ts'o <tytso@xxxxxxx> wrote:
>
> Thus, add an optional configuration option which stops getrandom(2)
> from blocking, but instead returns "best efforts" randomness, which
> might not be random or secure at all.

So I hate having a config option for something like this.

How about this attached patch instead? It only changes the waiting
logic, and I'll quote the comment in full, because I think that
explains not only the rationale, it explains every part of the patch
(and is most of the patch anyway):

 * We refuse to wait very long for a blocking getrandom().
 *
 * The crng may not be ready during boot, but if you ask for
 * blocking random numbers very early, there is no guarantee
 * that you'll ever get any timely entropy.
 *
 * If you are sure you need entropy and that you can generate
 * it, you need to ask for non-blocking random state, and then
 * if that fails you must actively _do_something_ that causes
 * enough system activity, perhaps asking the user to type
 * something on the keyboard.
 *
 * Just asking for blocking random numbers is completely and
 * fundamentally wrong, and the kernel will not play that game.
 *
 * We will block for at most 15 seconds at a time, and if called
 * sequentially will decrease the blocking amount so that we'll
 * block for at most 30s total - and if people continue to ask
 * for blocking, at that point we'll just return whatever random
 * state we have acquired.
 *
 * This will also complain loudly if the timeout happens, to let
 * the distribution or system admin know about the problem.
 *
 * The process that gets the -EAGAIN will hopefully also log the
 * error, to raise awareness that there may be use of random
 * numbers without sufficient entropy.

Hmm? No strange behavior. No odd config variables. A bounded total
boot-time wait of 30s (which is a completely random number, but I
claimed it as the "big red button" time).

And if you only do it once and fall back to something else it will
only wait for 15s, and you'll have your error value so that you can
log it properly.

Yes, a single boot-time wait of 15s at boot is still "darn annoying",
but it likely

 (a) isn't so long that people consider it a boot failure and give up
(but hopefully annoying enough that they'll report it)

 (b) long enough that *if* the thing that is waiting is not actually
blocking the boot sequence, the non-blocked part of the boot sequence
should have time to do sufficient IO to get better randomness.

So (a) is the "the system is still usable" part. While (b) is the
"give it a chance, and even if it fails and you fall back on urandom
or whatever, you'll actually be getting good randomness even if we
can't perhaps _guarantee_ entropy".

Also, if you have some user that wants to do the old-timey ssh-keygen
thing with user input etc, we now have a documented way to do that:
just do the nonblocking thing, and then make really really sure that
you actually have something that generates more entropy if that
nonblocking thing returns EAGAIN. But it's also very clear that at
that point the program that wants this entropy guarantee has to _work_
for it.

Because just being lazy and say "block" without any entropy will
return EAGAIN for a (continually decreasing) while, but then at some
point stop and say "you're broken", and just give you the urandom
data.

Because if you really do nothing at all, and there is no activity
what-so-ever for 15s because you blocked the boot, then I claim that
it's better to return an error than to wait forever. And if you ignore
the error and just retry, eventually we'll do the fallback for you.

Of course, if you have something like rdrand, and told us you trust
it, none of this matters at all, since we'll have initialized the pool
long before.

So this is unconditional, but it's basically "unconditionally somewhat
flexibly reasonable". It should only ever trigger for the case where
the boot sequence was fundamentally broken. And it will complain
loudly (both at a kernel level, and hopefully at a systemd journal
level too) if it ever triggers.

And hey, if some distro wants to then revert this because they feel
uncomfortable with this, that's now _their_ problem, not the problem
of the upstream kernel. The upstream kernel tries to do something that
I think is arguably fairly reasonable in all situations.

                 Linus
 drivers/char/random.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 52 insertions(+), 3 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 5d5ea4ce1442..0d7dc86e0f60 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -2118,6 +2118,55 @@ const struct file_operations urandom_fops = {
 	.llseek = noop_llseek,
 };
 
+/*
+ * We refuse to wait very long for a blocking getrandom().
+ *
+ * The crng may not be ready during boot, but if you ask for
+ * blocking random numbers very early, there is no guarantee
+ * that you'll ever get any timely entropy.
+ *
+ * If you are sure you need entropy and that you can generate
+ * it, you need to ask for non-blocking random state, and then
+ * if that fails you must actively _do_something_ that causes
+ * enough system activity, perhaps asking the user to type
+ * something on the keyboard.
+ *
+ * Just asking for blocking random numbers is completely and
+ * fundamentally wrong, and the kernel will not play that game.
+ *
+ * We will block for at most 15 seconds at a time, and if called
+ * sequentially will decrease the blocking amount so that we'll
+ * block for at most 30s total - and if people continue to ask
+ * for blocking, at that point we'll just return whatever random
+ * state we have acquired.
+ *
+ * This will also complain loudly if the timeout happens, to let
+ * the distribution or system admin know about the problem.
+ *
+ * The process that gets the -EAGAIN will hopefully also log the
+ * error, to raise awareness that there may be use of random
+ * numbers without sufficient entropy.
+ */
+static int getrandom_wait(void)
+{
+	static unsigned int getrandom_timeout = 15*HZ;
+	unsigned int timeout = READ_ONCE(getrandom_timeout);
+	int ret;
+
+	/* "At some point you just have to give up on broken boot scripts" */
+	if (!timeout)
+		return 0;
+
+	ret = wait_event_interruptible_timeout(crng_init_wait, crng_ready(), timeout);
+	/* Success (>0) or interrupted (<0)? */
+	if (likely(ret))
+		return ret > 0 ? 0 : ret;
+
+	WRITE_ONCE(getrandom_timeout, timeout >> 1);
+	WARN_ONCE(1, "getrandom() timed out with no entropy");
+	return -EAGAIN;
+}
+
 SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
 		unsigned int, flags)
 {
@@ -2132,11 +2181,11 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
 	if (flags & GRND_RANDOM)
 		return _random_read(flags & GRND_NONBLOCK, buf, count);
 
-	if (!crng_ready()) {
+	if (unlikely(!crng_ready())) {
 		if (flags & GRND_NONBLOCK)
 			return -EAGAIN;
-		ret = wait_for_random_bytes();
-		if (unlikely(ret))
+		ret = getrandom_wait();
+		if (ret)
 			return ret;
 	}
 	return urandom_read(NULL, buf, count, NULL);