[PATCH] random: urandom reads block when CRNG is not initialized.

[PATCH] random: urandom reads block when CRNG is not initialized.

[Naveen Nathan]

Adds a compile-time option to ensure urandom reads block until
the cryptographic random number generator (CRNG) is initialized.

This fixes a long standing security issue, the so called boot-time
entropy hole, where systems (particularly headless and embededd)
generate cryptographic keys before the CRNG has been iniitalised,
as exhibited in the work at https://factorable.net/.

This is deliberately a compile-time option without a corresponding
command line option to toggle urandom blocking behavior to prevent
system builders shooting themselves in the foot by
accidently/deliberately/maliciously toggling the option off in
production builds.

Signed-off-by: Naveen Nathan <naveen@lastninja.net>
---
 drivers/char/Kconfig  |  9 ++++++++
 drivers/char/random.c | 48 +++++++++++++++++++++++++++++++++++--------
 2 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 466ebd84ad17..9a09fdb37040 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -559,6 +559,15 @@ config ADI
 
 endmenu
 
+config ALWAYS_SECURE_URANDOM
+	bool "Ensure /dev/urandom always produces secure randomness"
+	default n
+	help
+	  Ensure reads to /dev/urandom block until Linux CRNG is initialized.
+	  All reads after initialization are non-blocking. This protects
+	  readers of /dev/urandom from receiving insecure randomness on cold
+	  start when the entropy pool isn't initially filled.
+
 config RANDOM_TRUST_CPU
 	bool "Trust the CPU manufacturer to initialize Linux's CRNG"
 	depends on X86 || S390 || PPC
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 5d5ea4ce1442..c2bca7fbca5e 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -473,6 +473,10 @@ static const struct poolinfo {
  */
 static DECLARE_WAIT_QUEUE_HEAD(random_read_wait);
 static DECLARE_WAIT_QUEUE_HEAD(random_write_wait);
+#if IS_ENABLED(CONFIG_ALWAYS_SECURE_URANDOM)
+static DECLARE_WAIT_QUEUE_HEAD(urandom_read_wait);
+static DECLARE_WAIT_QUEUE_HEAD(urandom_write_wait);
+#endif
 static struct fasync_struct *fasync;
 
 static DEFINE_SPINLOCK(random_ready_list_lock);
@@ -1966,15 +1970,23 @@ urandom_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)
 	static int maxwarn = 10;
 	int ret;
 
-	if (!crng_ready() && maxwarn > 0) {
-		maxwarn--;
-		if (__ratelimit(&urandom_warning))
-			printk(KERN_NOTICE "random: %s: uninitialized "
-			       "urandom read (%zd bytes read)\n",
-			       current->comm, nbytes);
-		spin_lock_irqsave(&primary_crng.lock, flags);
-		crng_init_cnt = 0;
-		spin_unlock_irqrestore(&primary_crng.lock, flags);
+	if (!crng_ready()) {
+		if (IS_ENABLED(CONFIG_ALWAYS_SECURE_URANDOM)) {
+			if (file->f_flags & O_NONBLOCK)
+				return -EAGAIN;
+			ret = wait_for_random_bytes();
+			if (unlikely(ret))
+				return ret;
+		} else if (maxwarn > 0) {
+			maxwarn--;
+			if (__ratelimit(&urandom_warning))
+				pr_notice("random: %s: uninitialized "
+				       "urandom read (%zd bytes read)\n",
+				       current->comm, nbytes);
+			spin_lock_irqsave(&primary_crng.lock, flags);
+			crng_init_cnt = 0;
+			spin_unlock_irqrestore(&primary_crng.lock, flags);
+		}
 	}
 	nbytes = min_t(size_t, nbytes, INT_MAX >> (ENTROPY_SHIFT + 3));
 	ret = extract_crng_user(buf, nbytes);
@@ -1997,6 +2009,21 @@ random_poll(struct file *file, poll_table * wait)
 	return mask;
 }
 
+#if IS_ENABLED(CONFIG_ALWAYS_SECURE_URANDOM)
+static __poll_t
+urandom_poll(struct file *file, poll_table *wait)
+{
+	__poll_t mask;
+
+	poll_wait(file, &urandom_read_wait, wait);
+	poll_wait(file, &urandom_write_wait, wait);
+	mask = EPOLLOUT | EPOLLWRNORM;
+	if (crng_ready())
+		mask |= EPOLLIN | EPOLLRDNORM;
+	return mask;
+}
+#endif
+
 static int
 write_pool(struct entropy_store *r, const char __user *buffer, size_t count)
 {
@@ -2113,6 +2140,9 @@ const struct file_operations random_fops = {
 const struct file_operations urandom_fops = {
 	.read  = urandom_read,
 	.write = random_write,
+#if IS_ENABLED(CONFIG_ALWAYS_SECURE_URANDOM)
+	.poll  = urandom_poll,
+#endif
 	.unlocked_ioctl = random_ioctl,
 	.fasync = random_fasync,
 	.llseek = noop_llseek,
-- 
2.17.1

Re: [PATCH] random: urandom reads block when CRNG is not initialized.

[Theodore Ts'o]

On Mon, May 27, 2019 at 12:26:28PM +0000, Naveen Nathan wrote:
> Adds a compile-time option to ensure urandom reads block until
> the cryptographic random number generator (CRNG) is initialized.
> 
> This fixes a long standing security issue, the so called boot-time
> entropy hole, where systems (particularly headless and embededd)
> generate cryptographic keys before the CRNG has been iniitalised,
> as exhibited in the work at https://factorable.net/.
> 
> This is deliberately a compile-time option without a corresponding
> command line option to toggle urandom blocking behavior to prevent
> system builders shooting themselves in the foot by
> accidently/deliberately/maliciously toggling the option off in
> production builds.
> 
> Signed-off-by: Naveen Nathan <naveen@lastninja.net>

This is guaranteed to cause the system to fail for systems using
systemd.  (Unless you are running an x86 with random.trust_cpu=1 ---
in which case, this patch/config is pointless.)  And many embedded
systems *do* use systemd.  I know lots of people like to wish that
systemd doesn't exist, but we need to face reality.

*Seriously,* if this is something the system builder should be using,
they should be fixing userspace.  And if they care enough that they
would want to enable this patch, they could just scan dmesg looking
for the warnings from the kernel.

						- Ted

Re: [PATCH] random: urandom reads block when CRNG is not initialized.

[Naveen Nathan]

On Mon, May 27, 2019 at 10:06:43AM -0400, Theodore Ts'o wrote:
>> [...] 
>
> This is guaranteed to cause the system to fail for systems using
> systemd.  (Unless you are running an x86 with random.trust_cpu=1 ---
> in which case, this patch/config is pointless.)  And many embedded
> systems *do* use systemd.  I know lots of people like to wish that
> systemd doesn't exist, but we need to face reality.

Hence a compile-time option; systemd systems need not use it yet.
I would argue systemd needs to fix their randomness API (it's a sad
joke), and use secure randomness only where required. For example,
it is said that systemd relies on randomness to generate unique UUIDs
where a UUIDv4 would suffice. I'm happy to add a disclaimer in the
kernel config that this will break systemd.

> *Seriously,* if this is something the system builder should be using,
> they should be fixing userspace.  And if they care enough that they
> would want to enable this patch, they could just scan dmesg looking
> for the warnings from the kernel.

And I think this is the more interesting case, system builders should
ideally fix userspace and rely on getrandom (which is no different to
this compile time option). But the reality is the boot entropy hole
problem has been the source of many insecure cryptographic keys, and
this provides a simple assurance that it can no longer be the cause
of these issues (supposing good entropy is gathered).

- Naveen

Re: [PATCH] random: urandom reads block when CRNG is not initialized.

[Jason A. Donenfeld]

On Mon, May 27, 2019 at 4:07 PM Theodore Ts'o <tytso@mit.edu> wrote:
>
> This is guaranteed to cause the system to fail for systems using
> systemd.  (Unless you are running an x86 with random.trust_cpu=1 ---
> in which case, this patch/config is pointless.)  And many embedded
> systems *do* use systemd.  I know lots of people like to wish that
> systemd doesn't exist, but we need to face reality.
>
> *Seriously,* if this is something the system builder should be using,
> they should be fixing userspace.  And if they care enough that they
> would want to enable this patch, they could just scan dmesg looking
> for the warnings from the kernel.

Really, it's a chicken & egg thing. If people who make userspaces
never have an option to design around the correct behavior, they'll
continue to rely on the broken behavior. But if we give them a way to
compile their kernels with the correct behavior, eventually some
userspaces will run fine with it. "But they should just use
getrandom()!" you shout. Yes, and maybe the code most userspace
builders provide does do this. But people like to plug-in plenty of
third party things into their userspaces, and I think there's some
value in a userspace being able to say, "we've dealt with the
/dev/urandom situation, and we now do the right thing, so we can
enable this option, and now the code you run on our userspace will
give good randomness."

More concretely, distros might ship an init system that allows
enabling this option without creating issues. Systemd might improve.
OpenRC and runit-based distros appear to exist. Some folks do very
custom stuff. If they manage to make it work with the correct urandom
behavior, then that's a good situation for everything else to build
on, and for providing better guarantees for third party software that
runs on that distro. But none of this is possible without the ability
to compile the kernel with the correct behavior.

Re: [PATCH] random: urandom reads block when CRNG is not initialized.

[Theodore Ts'o]

On Mon, May 27, 2019 at 05:43:03PM +0200, Jason A. Donenfeld wrote:
> 
> Really, it's a chicken & egg thing. If people who make userspaces
> never have an option to design around the correct behavior, they'll
> continue to rely on the broken behavior. But if we give them a way to
> compile their kernels with the correct behavior, eventually some
> userspaces will run fine with it. "But they should just use
> getrandom()!" you shout. Yes, and maybe the code most userspace
> builders provide does do this. But people like to plug-in plenty of
> third party things into their userspaces, and I think there's some
> value in a userspace being able to say, "we've dealt with the
> /dev/urandom situation, and we now do the right thing, so we can
> enable this option, and now the code you run on our userspace will
> give good randomness."

The challenge is that in some cases, there *is* no good solution.  If
you don't have decent hardware support, there's not a whole lot you
can do as a systems integrator or the distro.  Enabling this feature
will be a support nightmare, so I predict that darned few
distributions will be to enable it.

At the very least, we need to document the reality as it exists today,
which is that systems with systemd and Openwrt (and its derivitives),
*will* hang if this option is selected.  And we need to make sure that
zero-day testing with randconfig doesn't try to select this option
(hiding it behind CONFIG_EXPERIMENTAL should do the trick), since we
won't want to get spammed by the zero-day test bot.  That's how much
this option is guaranteed to break systems --- our continuous
integration testing systems are guaranteed to trip against it.

	    	    	    		   - Ted