linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] Rework random blocking
@ 2019-08-30  1:11 Andy Lutomirski
  2019-08-30  1:11 ` [PATCH 1/7] random: Don't wake crng_init_wait when crng_init == 1 Andy Lutomirski
                   ` (8 more replies)
  0 siblings, 9 replies; 12+ messages in thread
From: Andy Lutomirski @ 2019-08-30  1:11 UTC (permalink / raw)
  To: Theodore Tso
  Cc: LKML, Linux API, Kees Cook, Jason A. Donenfeld, Andy Lutomirski

This makes two major semantic changes to Linux's random APIs:

It adds getentropy(..., GRND_INSECURE).  This causes getentropy to
always return *something*.  There is no guarantee whatsoever that
the result will be cryptographically random or even unique, but the
kernel will give the best quality random output it can.  The name is
a big hint: the resulting output is INSECURE.

The purpose of this is to allow programs that genuinely want
best-effort entropy to get it without resorting to /dev/urandom.
Plenty of programs do this because they need to do *something*
during boot and they can't afford to wait.  Calling it "INSECURE" is
probably the best we can do to discourage using this API for things
that need security.

This series also removes the blocking pool and makes /dev/random
work just like getentropy(..., 0) and makes GRND_RANDOM a no-op.  I
believe that Linux's blocking pool has outlived its usefulness.
Linux's CRNG generates output that is good enough to use even for
key generation.  The blocking pool is not stronger in any material
way, and keeping it around requires a lot of infrastructure of
dubious value.

This series should not break any existing programs.  /dev/urandom is
unchanged.  /dev/random will still block just after booting, but it
will block less than it used to.  getentropy() with existing flags
will return output that is, for practical purposes, just as strong
as before.

Andy Lutomirski (7):
  random: Don't wake crng_init_wait when crng_init == 1
  random: Add GRND_INSECURE to return best-effort non-cryptographic
    bytes
  random: Ignore GRND_RANDOM in getentropy(2)
  random: Make /dev/random be almost like /dev/urandom
  random: Remove the blocking pool
  random: Delete code to pull data into pools
  random: Remove kernel.random.read_wakeup_threshold

 drivers/char/random.c       | 234 ++++--------------------------------
 include/uapi/linux/random.h |   4 +-
 2 files changed, 27 insertions(+), 211 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/7] random: Don't wake crng_init_wait when crng_init == 1
  2019-08-30  1:11 [PATCH 0/7] Rework random blocking Andy Lutomirski
@ 2019-08-30  1:11 ` Andy Lutomirski
  2019-08-30  1:11 ` [PATCH 2/7] random: Add GRND_INSECURE to return best-effort non-cryptographic bytes Andy Lutomirski
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Andy Lutomirski @ 2019-08-30  1:11 UTC (permalink / raw)
  To: Theodore Tso
  Cc: LKML, Linux API, Kees Cook, Jason A. Donenfeld, Andy Lutomirski

crng_init_wait is only used to wayt for crng_init to be set to 2, so
there's no point to waking it when crng_init is set to 1.  Remove the
unnecessary wake_up_interruptible() call.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 drivers/char/random.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 5d5ea4ce1442..d152612e08fc 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -946,7 +946,6 @@ static int crng_fast_load(const char *cp, size_t len)
 	if (crng_init_cnt >= CRNG_INIT_CNT_THRESH) {
 		invalidate_batched_entropy();
 		crng_init = 1;
-		wake_up_interruptible(&crng_init_wait);
 		pr_notice("random: fast init done\n");
 	}
 	return 1;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/7] random: Add GRND_INSECURE to return best-effort non-cryptographic bytes
  2019-08-30  1:11 [PATCH 0/7] Rework random blocking Andy Lutomirski
  2019-08-30  1:11 ` [PATCH 1/7] random: Don't wake crng_init_wait when crng_init == 1 Andy Lutomirski
@ 2019-08-30  1:11 ` Andy Lutomirski
  2019-08-30  1:11 ` [PATCH 3/7] random: Ignore GRND_RANDOM in getentropy(2) Andy Lutomirski
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Andy Lutomirski @ 2019-08-30  1:11 UTC (permalink / raw)
  To: Theodore Tso
  Cc: LKML, Linux API, Kees Cook, Jason A. Donenfeld, Andy Lutomirski

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 drivers/char/random.c       | 11 +++++++++--
 include/uapi/linux/random.h |  2 ++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index d152612e08fc..acabb870f222 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -2122,7 +2122,14 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
 {
 	int ret;
 
-	if (flags & ~(GRND_NONBLOCK|GRND_RANDOM))
+	if (flags & ~(GRND_NONBLOCK|GRND_RANDOM|GRND_INSECURE))
+		return -EINVAL;
+
+	/*
+	 * Requesting insecure and blocking randomness at the same time makes
+	 * no sense.
+	 */
+	if ((flags & (GRND_INSECURE|GRND_RANDOM)) == (GRND_INSECURE|GRND_RANDOM))
 		return -EINVAL;
 
 	if (count > INT_MAX)
@@ -2131,7 +2138,7 @@ 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 (!(flags & GRND_INSECURE) && !crng_ready()) {
 		if (flags & GRND_NONBLOCK)
 			return -EAGAIN;
 		ret = wait_for_random_bytes();
diff --git a/include/uapi/linux/random.h b/include/uapi/linux/random.h
index 26ee91300e3e..c092d20088d3 100644
--- a/include/uapi/linux/random.h
+++ b/include/uapi/linux/random.h
@@ -49,8 +49,10 @@ struct rand_pool_info {
  *
  * GRND_NONBLOCK	Don't block and return EAGAIN instead
  * GRND_RANDOM		Use the /dev/random pool instead of /dev/urandom
+ * GRND_INSECURE	Return non-cryptographic random bytes
  */
 #define GRND_NONBLOCK	0x0001
 #define GRND_RANDOM	0x0002
+#define GRND_INSECURE	0x0004
 
 #endif /* _UAPI_LINUX_RANDOM_H */
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/7] random: Ignore GRND_RANDOM in getentropy(2)
  2019-08-30  1:11 [PATCH 0/7] Rework random blocking Andy Lutomirski
  2019-08-30  1:11 ` [PATCH 1/7] random: Don't wake crng_init_wait when crng_init == 1 Andy Lutomirski
  2019-08-30  1:11 ` [PATCH 2/7] random: Add GRND_INSECURE to return best-effort non-cryptographic bytes Andy Lutomirski
@ 2019-08-30  1:11 ` Andy Lutomirski
  2019-08-30  1:11 ` [PATCH 4/7] random: Make /dev/random be almost like /dev/urandom Andy Lutomirski
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Andy Lutomirski @ 2019-08-30  1:11 UTC (permalink / raw)
  To: Theodore Tso
  Cc: LKML, Linux API, Kees Cook, Jason A. Donenfeld, Andy Lutomirski

The separate blocking pool is going away.  Start by ignoring
GRND_RANDOM in getentropy(2).

This should not materially break any API.  Any code that worked
without this change should work at least as well with this change.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 drivers/char/random.c       | 3 ---
 include/uapi/linux/random.h | 2 +-
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index acabb870f222..1ad2c7eaf675 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -2135,9 +2135,6 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
 	if (count > INT_MAX)
 		count = INT_MAX;
 
-	if (flags & GRND_RANDOM)
-		return _random_read(flags & GRND_NONBLOCK, buf, count);
-
 	if (!(flags & GRND_INSECURE) && !crng_ready()) {
 		if (flags & GRND_NONBLOCK)
 			return -EAGAIN;
diff --git a/include/uapi/linux/random.h b/include/uapi/linux/random.h
index c092d20088d3..dcc1b3e6106f 100644
--- a/include/uapi/linux/random.h
+++ b/include/uapi/linux/random.h
@@ -48,7 +48,7 @@ struct rand_pool_info {
  * Flags for getrandom(2)
  *
  * GRND_NONBLOCK	Don't block and return EAGAIN instead
- * GRND_RANDOM		Use the /dev/random pool instead of /dev/urandom
+ * GRND_RANDOM		No effect
  * GRND_INSECURE	Return non-cryptographic random bytes
  */
 #define GRND_NONBLOCK	0x0001
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/7] random: Make /dev/random be almost like /dev/urandom
  2019-08-30  1:11 [PATCH 0/7] Rework random blocking Andy Lutomirski
                   ` (2 preceding siblings ...)
  2019-08-30  1:11 ` [PATCH 3/7] random: Ignore GRND_RANDOM in getentropy(2) Andy Lutomirski
@ 2019-08-30  1:11 ` Andy Lutomirski
  2019-08-30  1:11 ` [PATCH 5/7] random: Remove the blocking pool Andy Lutomirski
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Andy Lutomirski @ 2019-08-30  1:11 UTC (permalink / raw)
  To: Theodore Tso
  Cc: LKML, Linux API, Kees Cook, Jason A. Donenfeld, Andy Lutomirski

This patch changes the read semantics of /dev/random to be the same
as /dev/urandom except that reads will block until the CRNG is
ready.

None of the cleanups that this enables have been done yet.  As a
result, this gives a warning about an unused function.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 drivers/char/random.c | 55 +++++++++++--------------------------------
 1 file changed, 14 insertions(+), 41 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 1ad2c7eaf675..29a158d9353c 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -354,7 +354,6 @@
 #define INPUT_POOL_WORDS	(1 << (INPUT_POOL_SHIFT-5))
 #define OUTPUT_POOL_SHIFT	10
 #define OUTPUT_POOL_WORDS	(1 << (OUTPUT_POOL_SHIFT-5))
-#define SEC_XFER_SIZE		512
 #define EXTRACT_SIZE		10
 
 
@@ -803,7 +802,6 @@ static void credit_entropy_bits(struct entropy_store *r, int nbits)
 		if (entropy_bits >= random_read_wakeup_bits &&
 		    wq_has_sleeper(&random_read_wait)) {
 			wake_up_interruptible(&random_read_wait);
-			kill_fasync(&fasync, SIGIO, POLL_IN);
 		}
 		/* If the input pool is getting full, and the blocking
 		 * pool has room, send some entropy to the blocking
@@ -1031,6 +1029,7 @@ static void crng_reseed(struct crng_state *crng, struct entropy_store *r)
 		crng_init = 2;
 		process_random_ready_list();
 		wake_up_interruptible(&crng_init_wait);
+		kill_fasync(&fasync, SIGIO, POLL_IN);
 		pr_notice("random: crng init done\n");
 		if (unseeded_warning.missed) {
 			pr_notice("random: %d get_random_xx warning(s) missed "
@@ -1921,43 +1920,6 @@ void rand_initialize_disk(struct gendisk *disk)
 }
 #endif
 
-static ssize_t
-_random_read(int nonblock, char __user *buf, size_t nbytes)
-{
-	ssize_t n;
-
-	if (nbytes == 0)
-		return 0;
-
-	nbytes = min_t(size_t, nbytes, SEC_XFER_SIZE);
-	while (1) {
-		n = extract_entropy_user(&blocking_pool, buf, nbytes);
-		if (n < 0)
-			return n;
-		trace_random_read(n*8, (nbytes-n)*8,
-				  ENTROPY_BITS(&blocking_pool),
-				  ENTROPY_BITS(&input_pool));
-		if (n > 0)
-			return n;
-
-		/* Pool is (near) empty.  Maybe wait and retry. */
-		if (nonblock)
-			return -EAGAIN;
-
-		wait_event_interruptible(random_read_wait,
-		    blocking_pool.initialized &&
-		    (ENTROPY_BITS(&input_pool) >= random_read_wakeup_bits));
-		if (signal_pending(current))
-			return -ERESTARTSYS;
-	}
-}
-
-static ssize_t
-random_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)
-{
-	return _random_read(file->f_flags & O_NONBLOCK, buf, nbytes);
-}
-
 static ssize_t
 urandom_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)
 {
@@ -1981,15 +1943,26 @@ urandom_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)
 	return ret;
 }
 
+static ssize_t
+random_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)
+{
+	int ret;
+
+	ret = wait_for_random_bytes();
+	if (ret != 0)
+		return ret;
+	return urandom_read(file, buf, nbytes, ppos);
+}
+
 static __poll_t
 random_poll(struct file *file, poll_table * wait)
 {
 	__poll_t mask;
 
-	poll_wait(file, &random_read_wait, wait);
+	poll_wait(file, &crng_init_wait, wait);
 	poll_wait(file, &random_write_wait, wait);
 	mask = 0;
-	if (ENTROPY_BITS(&input_pool) >= random_read_wakeup_bits)
+	if (crng_ready())
 		mask |= EPOLLIN | EPOLLRDNORM;
 	if (ENTROPY_BITS(&input_pool) < random_write_wakeup_bits)
 		mask |= EPOLLOUT | EPOLLWRNORM;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/7] random: Remove the blocking pool
  2019-08-30  1:11 [PATCH 0/7] Rework random blocking Andy Lutomirski
                   ` (3 preceding siblings ...)
  2019-08-30  1:11 ` [PATCH 4/7] random: Make /dev/random be almost like /dev/urandom Andy Lutomirski
@ 2019-08-30  1:11 ` Andy Lutomirski
  2019-08-30  1:11 ` [PATCH 6/7] random: Delete code to pull data into pools Andy Lutomirski
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Andy Lutomirski @ 2019-08-30  1:11 UTC (permalink / raw)
  To: Theodore Tso
  Cc: LKML, Linux API, Kees Cook, Jason A. Donenfeld, Andy Lutomirski

There is no longer any interface to read data from the blocking
pool, so remove it.

This enables quite a bit of code deletion, much of which will be
done in subsequent patches.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 drivers/char/random.c | 106 ------------------------------------------
 1 file changed, 106 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 29a158d9353c..4521138231ed 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -470,7 +470,6 @@ static const struct poolinfo {
 /*
  * Static global variables
  */
-static DECLARE_WAIT_QUEUE_HEAD(random_read_wait);
 static DECLARE_WAIT_QUEUE_HEAD(random_write_wait);
 static struct fasync_struct *fasync;
 
@@ -530,7 +529,6 @@ struct entropy_store {
 	__u32 *pool;
 	const char *name;
 	struct entropy_store *pull;
-	struct work_struct push_work;
 
 	/* read-write data: */
 	unsigned long last_pulled;
@@ -549,9 +547,7 @@ static ssize_t _extract_entropy(struct entropy_store *r, void *buf,
 				size_t nbytes, int fips);
 
 static void crng_reseed(struct crng_state *crng, struct entropy_store *r);
-static void push_to_pool(struct work_struct *work);
 static __u32 input_pool_data[INPUT_POOL_WORDS] __latent_entropy;
-static __u32 blocking_pool_data[OUTPUT_POOL_WORDS] __latent_entropy;
 
 static struct entropy_store input_pool = {
 	.poolinfo = &poolinfo_table[0],
@@ -560,16 +556,6 @@ static struct entropy_store input_pool = {
 	.pool = input_pool_data
 };
 
-static struct entropy_store blocking_pool = {
-	.poolinfo = &poolinfo_table[1],
-	.name = "blocking",
-	.pull = &input_pool,
-	.lock = __SPIN_LOCK_UNLOCKED(blocking_pool.lock),
-	.pool = blocking_pool_data,
-	.push_work = __WORK_INITIALIZER(blocking_pool.push_work,
-					push_to_pool),
-};
-
 static __u32 const twist_table[8] = {
 	0x00000000, 0x3b6e20c8, 0x76dc4190, 0x4db26158,
 	0xedb88320, 0xd6d6a3e8, 0x9b64c2b0, 0xa00ae278 };
@@ -765,15 +751,11 @@ static void credit_entropy_bits(struct entropy_store *r, int nbits)
 		entropy_count = 0;
 	} else if (entropy_count > pool_size)
 		entropy_count = pool_size;
-	if ((r == &blocking_pool) && !r->initialized &&
-	    (entropy_count >> ENTROPY_SHIFT) > 128)
-		has_initialized = 1;
 	if (cmpxchg(&r->entropy_count, orig, entropy_count) != orig)
 		goto retry;
 
 	if (has_initialized) {
 		r->initialized = 1;
-		wake_up_interruptible(&random_read_wait);
 		kill_fasync(&fasync, SIGIO, POLL_IN);
 	}
 
@@ -782,7 +764,6 @@ static void credit_entropy_bits(struct entropy_store *r, int nbits)
 
 	if (r == &input_pool) {
 		int entropy_bits = entropy_count >> ENTROPY_SHIFT;
-		struct entropy_store *other = &blocking_pool;
 
 		if (crng_init < 2) {
 			if (entropy_bits < 128)
@@ -790,27 +771,6 @@ static void credit_entropy_bits(struct entropy_store *r, int nbits)
 			crng_reseed(&primary_crng, r);
 			entropy_bits = r->entropy_count >> ENTROPY_SHIFT;
 		}
-
-		/* initialize the blocking pool if necessary */
-		if (entropy_bits >= random_read_wakeup_bits &&
-		    !other->initialized) {
-			schedule_work(&other->push_work);
-			return;
-		}
-
-		/* should we wake readers? */
-		if (entropy_bits >= random_read_wakeup_bits &&
-		    wq_has_sleeper(&random_read_wait)) {
-			wake_up_interruptible(&random_read_wait);
-		}
-		/* If the input pool is getting full, and the blocking
-		 * pool has room, send some entropy to the blocking
-		 * pool.
-		 */
-		if (!work_pending(&other->push_work) &&
-		    (ENTROPY_BITS(r) > 6 * r->poolinfo->poolbytes) &&
-		    (ENTROPY_BITS(other) <= 6 * other->poolinfo->poolbytes))
-			schedule_work(&other->push_work);
 	}
 }
 
@@ -1422,22 +1382,6 @@ static void _xfer_secondary_pool(struct entropy_store *r, size_t nbytes)
 	credit_entropy_bits(r, bytes*8);
 }
 
-/*
- * Used as a workqueue function so that when the input pool is getting
- * full, we can "spill over" some entropy to the output pools.  That
- * way the output pools can store some of the excess entropy instead
- * of letting it go to waste.
- */
-static void push_to_pool(struct work_struct *work)
-{
-	struct entropy_store *r = container_of(work, struct entropy_store,
-					      push_work);
-	BUG_ON(!r);
-	_xfer_secondary_pool(r, random_read_wakeup_bits/8);
-	trace_push_to_pool(r->name, r->entropy_count >> ENTROPY_SHIFT,
-			   r->pull->entropy_count >> ENTROPY_SHIFT);
-}
-
 /*
  * This function decides how many bytes to actually take from the
  * given pool, and also debits the entropy count accordingly.
@@ -1616,54 +1560,6 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf,
 	return _extract_entropy(r, buf, nbytes, fips_enabled);
 }
 
-/*
- * This function extracts randomness from the "entropy pool", and
- * returns it in a userspace buffer.
- */
-static ssize_t extract_entropy_user(struct entropy_store *r, void __user *buf,
-				    size_t nbytes)
-{
-	ssize_t ret = 0, i;
-	__u8 tmp[EXTRACT_SIZE];
-	int large_request = (nbytes > 256);
-
-	trace_extract_entropy_user(r->name, nbytes, ENTROPY_BITS(r), _RET_IP_);
-	if (!r->initialized && r->pull) {
-		xfer_secondary_pool(r, ENTROPY_BITS(r->pull)/8);
-		if (!r->initialized)
-			return 0;
-	}
-	xfer_secondary_pool(r, nbytes);
-	nbytes = account(r, nbytes, 0, 0);
-
-	while (nbytes) {
-		if (large_request && need_resched()) {
-			if (signal_pending(current)) {
-				if (ret == 0)
-					ret = -ERESTARTSYS;
-				break;
-			}
-			schedule();
-		}
-
-		extract_buf(r, tmp);
-		i = min_t(int, nbytes, EXTRACT_SIZE);
-		if (copy_to_user(buf, tmp, i)) {
-			ret = -EFAULT;
-			break;
-		}
-
-		nbytes -= i;
-		buf += i;
-		ret += i;
-	}
-
-	/* Wipe data just returned from memory */
-	memzero_explicit(tmp, sizeof(tmp));
-
-	return ret;
-}
-
 #define warn_unseeded_randomness(previous) \
 	_warn_unseeded_randomness(__func__, (void *) _RET_IP_, (previous))
 
@@ -1893,7 +1789,6 @@ static void __init init_std_data(struct entropy_store *r)
 int __init rand_initialize(void)
 {
 	init_std_data(&input_pool);
-	init_std_data(&blocking_pool);
 	crng_initialize(&primary_crng);
 	crng_global_init_time = jiffies;
 	if (ratelimit_disable) {
@@ -2053,7 +1948,6 @@ static long random_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
 		if (!capable(CAP_SYS_ADMIN))
 			return -EPERM;
 		input_pool.entropy_count = 0;
-		blocking_pool.entropy_count = 0;
 		return 0;
 	case RNDRESEEDCRNG:
 		if (!capable(CAP_SYS_ADMIN))
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 6/7] random: Delete code to pull data into pools
  2019-08-30  1:11 [PATCH 0/7] Rework random blocking Andy Lutomirski
                   ` (4 preceding siblings ...)
  2019-08-30  1:11 ` [PATCH 5/7] random: Remove the blocking pool Andy Lutomirski
@ 2019-08-30  1:11 ` Andy Lutomirski
  2019-08-30  1:11 ` [PATCH 7/7] random: Remove kernel.random.read_wakeup_threshold Andy Lutomirski
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Andy Lutomirski @ 2019-08-30  1:11 UTC (permalink / raw)
  To: Theodore Tso
  Cc: LKML, Linux API, Kees Cook, Jason A. Donenfeld, Andy Lutomirski

There is no pool that pulls, so it was just dead code.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 drivers/char/random.c | 40 ----------------------------------------
 1 file changed, 40 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 4521138231ed..99fea5cc29a8 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -528,10 +528,8 @@ struct entropy_store {
 	const struct poolinfo *poolinfo;
 	__u32 *pool;
 	const char *name;
-	struct entropy_store *pull;
 
 	/* read-write data: */
-	unsigned long last_pulled;
 	spinlock_t lock;
 	unsigned short add_ptr;
 	unsigned short input_rotate;
@@ -1347,41 +1345,6 @@ EXPORT_SYMBOL_GPL(add_disk_randomness);
  *
  *********************************************************************/
 
-/*
- * This utility inline function is responsible for transferring entropy
- * from the primary pool to the secondary extraction pool. We make
- * sure we pull enough for a 'catastrophic reseed'.
- */
-static void _xfer_secondary_pool(struct entropy_store *r, size_t nbytes);
-static void xfer_secondary_pool(struct entropy_store *r, size_t nbytes)
-{
-	if (!r->pull ||
-	    r->entropy_count >= (nbytes << (ENTROPY_SHIFT + 3)) ||
-	    r->entropy_count > r->poolinfo->poolfracbits)
-		return;
-
-	_xfer_secondary_pool(r, nbytes);
-}
-
-static void _xfer_secondary_pool(struct entropy_store *r, size_t nbytes)
-{
-	__u32	tmp[OUTPUT_POOL_WORDS];
-
-	int bytes = nbytes;
-
-	/* pull at least as much as a wakeup */
-	bytes = max_t(int, bytes, random_read_wakeup_bits / 8);
-	/* but never more than the buffer size */
-	bytes = min_t(int, bytes, sizeof(tmp));
-
-	trace_xfer_secondary_pool(r->name, bytes * 8, nbytes * 8,
-				  ENTROPY_BITS(r), ENTROPY_BITS(r->pull));
-	bytes = extract_entropy(r->pull, tmp, bytes,
-				random_read_wakeup_bits / 8, 0);
-	mix_pool_bytes(r, tmp, bytes);
-	credit_entropy_bits(r, bytes*8);
-}
-
 /*
  * This function decides how many bytes to actually take from the
  * given pool, and also debits the entropy count accordingly.
@@ -1545,7 +1508,6 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf,
 			spin_unlock_irqrestore(&r->lock, flags);
 			trace_extract_entropy(r->name, EXTRACT_SIZE,
 					      ENTROPY_BITS(r), _RET_IP_);
-			xfer_secondary_pool(r, EXTRACT_SIZE);
 			extract_buf(r, tmp);
 			spin_lock_irqsave(&r->lock, flags);
 			memcpy(r->last_data, tmp, EXTRACT_SIZE);
@@ -1554,7 +1516,6 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf,
 	}
 
 	trace_extract_entropy(r->name, nbytes, ENTROPY_BITS(r), _RET_IP_);
-	xfer_secondary_pool(r, nbytes);
 	nbytes = account(r, nbytes, min, reserved);
 
 	return _extract_entropy(r, buf, nbytes, fips_enabled);
@@ -1765,7 +1726,6 @@ static void __init init_std_data(struct entropy_store *r)
 	ktime_t now = ktime_get_real();
 	unsigned long rv;
 
-	r->last_pulled = jiffies;
 	mix_pool_bytes(r, &now, sizeof(now));
 	for (i = r->poolinfo->poolbytes; i > 0; i -= sizeof(rv)) {
 		if (!arch_get_random_seed_long(&rv) &&
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 7/7] random: Remove kernel.random.read_wakeup_threshold
  2019-08-30  1:11 [PATCH 0/7] Rework random blocking Andy Lutomirski
                   ` (5 preceding siblings ...)
  2019-08-30  1:11 ` [PATCH 6/7] random: Delete code to pull data into pools Andy Lutomirski
@ 2019-08-30  1:11 ` Andy Lutomirski
  2019-08-30  1:49 ` [PATCH 0/7] Rework random blocking Theodore Y. Ts'o
  2019-09-09  9:42 ` Pavel Machek
  8 siblings, 0 replies; 12+ messages in thread
From: Andy Lutomirski @ 2019-08-30  1:11 UTC (permalink / raw)
  To: Theodore Tso
  Cc: LKML, Linux API, Kees Cook, Jason A. Donenfeld, Andy Lutomirski

It has no effect any more, so remove it.  We can revert this if
there is some user code that expects to be able to set this sysctl.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 drivers/char/random.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 99fea5cc29a8..2a284f30cac4 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -369,12 +369,6 @@
 #define ENTROPY_SHIFT 3
 #define ENTROPY_BITS(r) ((r)->entropy_count >> ENTROPY_SHIFT)
 
-/*
- * The minimum number of bits of entropy before we wake up a read on
- * /dev/random.  Should be enough to do a significant reseed.
- */
-static int random_read_wakeup_bits = 64;
-
 /*
  * If the entropy count falls under this number of bits, then we
  * should wake up processes which are selecting or polling on write
@@ -1982,8 +1976,7 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
 
 #include <linux/sysctl.h>
 
-static int min_read_thresh = 8, min_write_thresh;
-static int max_read_thresh = OUTPUT_POOL_WORDS * 32;
+static int min_write_thresh;
 static int max_write_thresh = INPUT_POOL_WORDS * 32;
 static int random_min_urandom_seed = 60;
 static char sysctl_bootid[16];
@@ -2058,15 +2051,6 @@ struct ctl_table random_table[] = {
 		.proc_handler	= proc_do_entropy,
 		.data		= &input_pool.entropy_count,
 	},
-	{
-		.procname	= "read_wakeup_threshold",
-		.data		= &random_read_wakeup_bits,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= &min_read_thresh,
-		.extra2		= &max_read_thresh,
-	},
 	{
 		.procname	= "write_wakeup_threshold",
 		.data		= &random_write_wakeup_bits,
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/7] Rework random blocking
  2019-08-30  1:11 [PATCH 0/7] Rework random blocking Andy Lutomirski
                   ` (6 preceding siblings ...)
  2019-08-30  1:11 ` [PATCH 7/7] random: Remove kernel.random.read_wakeup_threshold Andy Lutomirski
@ 2019-08-30  1:49 ` Theodore Y. Ts'o
  2019-08-30  2:01   ` Andy Lutomirski
  2019-09-09  9:42 ` Pavel Machek
  8 siblings, 1 reply; 12+ messages in thread
From: Theodore Y. Ts'o @ 2019-08-30  1:49 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Theodore Tso, LKML, Linux API, Kees Cook, Jason A. Donenfeld

On Thu, Aug 29, 2019 at 06:11:35PM -0700, Andy Lutomirski wrote:
> This series also removes the blocking pool and makes /dev/random
> work just like getentropy(..., 0) and makes GRND_RANDOM a no-op.  I
> believe that Linux's blocking pool has outlived its usefulness.
> Linux's CRNG generates output that is good enough to use even for
> key generation.  The blocking pool is not stronger in any material
> way, and keeping it around requires a lot of infrastructure of
> dubious value.

It's too late for the 5.4 cycle for a change of this magnitude, and
I'd just as soon let this wait until *after* the LTS kernel gets cut.
The reason for this is because at the moment, there are some PCI
compliance labs who believe that the "true randomness" of /dev/random
is necessary for PCI compliance and so they mandate the use of
/dev/random over /dev/urandom's "cryptographic randomness" for that
reason.  A lot of things which are thought to be needed for PCI
compliance that are about as useful as eye of newt and toe of frog,
but nothing says that PCI compliance (and enterprise customer
requirements :-) have to make sense.

It may be that what we might need to really support people (or stupid
compliance labs) who have a fetish for "true randomness" to get a
better interface for hardware random number generators than
/dev/hwrng.  Specifically, one which allows for a more sane way of
selecting which hardware random number generator to use if there are
multiple available, and also one where we mix in some CRNG as a
whitening step just case the hardware number generator is busted in
some way.  (And to fix the issue that at the moment, if someone evil
fakes up a USB device with the USB manufacturer and minor device
number for a ChosKey device that generates a insecure sequence, it
will still get blindly trusted by the kernel without any kind of
authentication of said hardware device.)

That probably means we need to come up with a new interface than
/dev/hwrng, or have some way of configuring /dev/random to use a
hardware RNG device for those people who really care about "true
randomness".  The current /dev/hwrng interface and how it is
configured via sysfs is pretty baroque IMO.

	      	  	     	       	  	 - Ted

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/7] Rework random blocking
  2019-08-30  1:49 ` [PATCH 0/7] Rework random blocking Theodore Y. Ts'o
@ 2019-08-30  2:01   ` Andy Lutomirski
  0 siblings, 0 replies; 12+ messages in thread
From: Andy Lutomirski @ 2019-08-30  2:01 UTC (permalink / raw)
  To: Theodore Y. Ts'o
  Cc: Andy Lutomirski, Theodore Tso, LKML, Linux API, Kees Cook,
	Jason A. Donenfeld



> On Aug 29, 2019, at 6:49 PM, Theodore Y. Ts'o <tytso@mit.edu> wrote:
> 
>> On Thu, Aug 29, 2019 at 06:11:35PM -0700, Andy Lutomirski wrote:
>> This series also removes the blocking pool and makes /dev/random
>> work just like getentropy(..., 0) and makes GRND_RANDOM a no-op.  I
>> believe that Linux's blocking pool has outlived its usefulness.
>> Linux's CRNG generates output that is good enough to use even for
>> key generation.  The blocking pool is not stronger in any material
>> way, and keeping it around requires a lot of infrastructure of
>> dubious value.
> 
> It's too late for the 5.4 cycle for a change of this magnitude, and
> I'd just as soon let this wait until *after* the LTS kernel gets cut.
> The reason for this is because at the moment, there are some PCI
> compliance labs who believe that the "true randomness" of /dev/random
> is necessary for PCI compliance and so they mandate the use of
> /dev/random over /dev/urandom's "cryptographic randomness" for that
> reason.  A lot of things which are thought to be needed for PCI
> compliance that are about as useful as eye of newt and toe of frog,
> but nothing says that PCI compliance (and enterprise customer
> requirements :-) have to make sense.
> 
> It may be that what we might need to really support people (or stupid
> compliance labs) who have a fetish for "true randomness" to get a
> better interface for hardware random number generators than
> /dev/hwrng.  Specifically, one which allows for a more sane way of
> selecting which hardware random number generator to use if there are
> multiple available, and also one where we mix in some CRNG as a
> whitening step just case the hardware number generator is busted in
> some way.  (And to fix the issue that at the moment, if someone evil
> fakes up a USB device with the USB manufacturer and minor device
> number for a ChosKey device that generates a insecure sequence, it
> will still get blindly trusted by the kernel without any kind of
> authentication of said hardware device.)
> 
> That probably means we need to come up with a new interface than
> /dev/hwrng, or have some way of configuring /dev/random to use a
> hardware RNG device for those people who really care about "true
> randomness".  The current /dev/hwrng interface and how it is
> configured via sysfs is pretty baroque IMO.
> 
>                     

Hmm. Does this really need to be in the kernel?  ISTM it should be straightforward to write a little CUSE program that grabs bytes from RDSEED or RDRAND, TPM, ChaosKey (if enabled, with a usb slot selected!), and whatever other sources are requested and, configurable to satisfy whoever actually cares, mixes some or all with a FIPS-compliant, provably-indististinguishable-from-random, definitely not Dual-EC mixer, and spits out the result.  And filters it and checks all the sources for credibility, and generally does whatever the user actually needs.

And the really over-the-top auditors can symlink it to /dev/random.

Do the PCI folks actually care that it’s in the kernel?


As an aside, the first two patches could plausibly land before the rest of the series if that seems appropriate.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/7] Rework random blocking
  2019-08-30  1:11 [PATCH 0/7] Rework random blocking Andy Lutomirski
                   ` (7 preceding siblings ...)
  2019-08-30  1:49 ` [PATCH 0/7] Rework random blocking Theodore Y. Ts'o
@ 2019-09-09  9:42 ` Pavel Machek
  2019-09-09 22:57   ` Andy Lutomirski
  8 siblings, 1 reply; 12+ messages in thread
From: Pavel Machek @ 2019-09-09  9:42 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Theodore Tso, LKML, Linux API, Kees Cook, Jason A. Donenfeld

[-- Attachment #1: Type: text/plain, Size: 1890 bytes --]

On Thu 2019-08-29 18:11:35, Andy Lutomirski wrote:
> This makes two major semantic changes to Linux's random APIs:
> 
> It adds getentropy(..., GRND_INSECURE).  This causes getentropy to
> always return *something*.  There is no guarantee whatsoever that
> the result will be cryptographically random or even unique, but the
> kernel will give the best quality random output it can.  The name is
> a big hint: the resulting output is INSECURE.
> 
> The purpose of this is to allow programs that genuinely want
> best-effort entropy to get it without resorting to /dev/urandom.
> Plenty of programs do this because they need to do *something*
> during boot and they can't afford to wait.  Calling it "INSECURE" is
> probably the best we can do to discourage using this API for things
> that need security.
> 
> This series also removes the blocking pool and makes /dev/random
> work just like getentropy(..., 0) and makes GRND_RANDOM a no-op.  I
> believe that Linux's blocking pool has outlived its usefulness.
> Linux's CRNG generates output that is good enough to use even for
> key generation.  The blocking pool is not stronger in any material
> way, and keeping it around requires a lot of infrastructure of
> dubious value.

Could you give some more justification? If crng is good enough for
you, you can use /dev/urandom...


are 

> This series should not break any existing programs.  /dev/urandom is
> unchanged.  /dev/random will still block just after booting, but it
> will block less than it used to.  getentropy() with existing flags
> will return output that is, for practical purposes, just as strong
> as before.

So what is the exact semantic of /dev/random after your change?
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/7] Rework random blocking
  2019-09-09  9:42 ` Pavel Machek
@ 2019-09-09 22:57   ` Andy Lutomirski
  0 siblings, 0 replies; 12+ messages in thread
From: Andy Lutomirski @ 2019-09-09 22:57 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Andy Lutomirski, Theodore Tso, LKML, Linux API, Kees Cook,
	Jason A. Donenfeld

On Mon, Sep 9, 2019 at 2:42 AM Pavel Machek <pavel@ucw.cz> wrote:
>
> On Thu 2019-08-29 18:11:35, Andy Lutomirski wrote:
> > This makes two major semantic changes to Linux's random APIs:
> >
> > It adds getentropy(..., GRND_INSECURE).  This causes getentropy to
> > always return *something*.  There is no guarantee whatsoever that
> > the result will be cryptographically random or even unique, but the
> > kernel will give the best quality random output it can.  The name is
> > a big hint: the resulting output is INSECURE.
> >
> > The purpose of this is to allow programs that genuinely want
> > best-effort entropy to get it without resorting to /dev/urandom.
> > Plenty of programs do this because they need to do *something*
> > during boot and they can't afford to wait.  Calling it "INSECURE" is
> > probably the best we can do to discourage using this API for things
> > that need security.
> >
> > This series also removes the blocking pool and makes /dev/random
> > work just like getentropy(..., 0) and makes GRND_RANDOM a no-op.  I
> > believe that Linux's blocking pool has outlived its usefulness.
> > Linux's CRNG generates output that is good enough to use even for
> > key generation.  The blocking pool is not stronger in any material
> > way, and keeping it around requires a lot of infrastructure of
> > dubious value.
>
> Could you give some more justification? If crng is good enough for
> you, you can use /dev/urandom...

Take a look at the diffstat.  The random code is extremely security
sensitive, and it's made considerably more complicated by the need to
support the blocking semantics for /dev/random.  My primary argument
is that there is no real reason for the kernel to continue to support
it.

>
>
> are
>
> > This series should not break any existing programs.  /dev/urandom is
> > unchanged.  /dev/random will still block just after booting, but it
> > will block less than it used to.  getentropy() with existing flags
> > will return output that is, for practical purposes, just as strong
> > as before.
>
> So what is the exact semantic of /dev/random after your change?

Reads return immediately if the CRNG is initialized, i.e reads return
immediately if and only if getentropy(..., 0) would succeed.
Otherwise reads block.

--Andy

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-09-09 22:58 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-30  1:11 [PATCH 0/7] Rework random blocking Andy Lutomirski
2019-08-30  1:11 ` [PATCH 1/7] random: Don't wake crng_init_wait when crng_init == 1 Andy Lutomirski
2019-08-30  1:11 ` [PATCH 2/7] random: Add GRND_INSECURE to return best-effort non-cryptographic bytes Andy Lutomirski
2019-08-30  1:11 ` [PATCH 3/7] random: Ignore GRND_RANDOM in getentropy(2) Andy Lutomirski
2019-08-30  1:11 ` [PATCH 4/7] random: Make /dev/random be almost like /dev/urandom Andy Lutomirski
2019-08-30  1:11 ` [PATCH 5/7] random: Remove the blocking pool Andy Lutomirski
2019-08-30  1:11 ` [PATCH 6/7] random: Delete code to pull data into pools Andy Lutomirski
2019-08-30  1:11 ` [PATCH 7/7] random: Remove kernel.random.read_wakeup_threshold Andy Lutomirski
2019-08-30  1:49 ` [PATCH 0/7] Rework random blocking Theodore Y. Ts'o
2019-08-30  2:01   ` Andy Lutomirski
2019-09-09  9:42 ` Pavel Machek
2019-09-09 22:57   ` Andy Lutomirski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).