[Resend PATCH 2/2] s390: provide hardware randomness from zcrypt card to /dev/random

[Resend PATCH 2/2] s390: provide hardware randomness from zcrypt card to /dev/random

[Torsten Duwe]

Running completely virtualised, system Z severely lacks good true random sources.
Gathering entropy in a virtual environment is difficult. To compensate, there is
specialised crypto hardware which includes a source for hardware randomness;
the zcrypt driver is able to access this random source. This patch adds a kernel
thread that feeds the random bits via the interface created with the previous patch.

Signed-off-by: Torsten Duwe <duwe@lst.de>

---
 zcrypt_api.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)
--- a/drivers/s390/crypto/zcrypt_api.c
+++ b/drivers/s390/crypto/zcrypt_api.c
@@ -38,6 +38,8 @@
 #include <linux/atomic.h>
 #include <asm/uaccess.h>
 #include <linux/hw_random.h>
+#include <linux/kthread.h>
+#include <linux/delay.h>
 #include <linux/debugfs.h>
 #include <asm/debug.h>
 
@@ -99,6 +99,13 @@ static ssize_t zcrypt_online_store(struc
 
 	if (sscanf(buf, "%d\n", &online) != 1 || online < 0 || online > 1)
 		return -EINVAL;
+	if (zdev->ops->rng) {
+		if (zdev->online == 0 && online == 1)
+			zcrypt_rng_device_add();
+		if (zdev->online == 1 && online == 0)
+			zcrypt_rng_device_remove();
+
+	}
 	zdev->online = online;
 	ZCRYPT_DBF_DEV(DBF_INFO, zdev, "dev%04xo%dman", zdev->ap_dev->qid,
 		       zdev->online);
@@ -1117,6 +1119,7 @@ static int zcrypt_rng_device_count;
 static u32 *zcrypt_rng_buffer;
 static int zcrypt_rng_buffer_index;
 static DEFINE_MUTEX(zcrypt_rng_mutex);
+static struct task_struct *zcrypt_hwrng_fill;
 
 static int zcrypt_rng_data_read(struct hwrng *rng, u32 *data)
 {
@@ -1141,6 +1144,36 @@ static struct hwrng zcrypt_rng_dev = {
 	.data_read	= zcrypt_rng_data_read,
 };
 
+static int zcrypt_hwrng_fillfn(void *unused)
+{
+	long rc;
+
+	while (!kthread_should_stop()) {
+		rc = zcrypt_rng((char *)zcrypt_rng_buffer);
+		if (rc == -ENODEV || rc == -EINVAL || rc == -ENOMEM) {
+			pr_err("zcrypt_rng unavailable: %ld\n", rc);
+			break;
+		}
+		if (rc == -EAGAIN || rc == -ERESTARTSYS) {
+			pr_info("zcrypt_rng interrupted: %ld\n", rc);
+			msleep_interruptible(1000);
+			continue;
+		}
+		if (rc == 0) {
+			pr_err("zcrypt_rng: no data available\n");
+			msleep_interruptible(10000);
+			continue;
+		}
+		if (rc < 0) {
+			pr_err("zcrypt_rng unknown error: %ld\n", rc);
+			break;
+		}
+		add_hwgenerator_randomness((void *)zcrypt_rng_buffer, rc);
+	}
+	zcrypt_hwrng_fill = 0;
+	return 0;
+}
+
 static int zcrypt_rng_device_add(void)
 {
 	int rc = 0;
@@ -1157,6 +1189,12 @@ static int zcrypt_rng_device_add(void)
 		if (rc)
 			goto out_free;
 		zcrypt_rng_device_count = 1;
+		zcrypt_hwrng_fill = kthread_run(zcrypt_hwrng_fillfn,
+			NULL, "zc_hwrng");
+		if (zcrypt_hwrng_fill == ERR_PTR(-ENOMEM)) {
+			pr_err("zcrypt_hwrng_fill thread creation failed\n");
+			zcrypt_hwrng_fill = 0;
+		}
 	} else
 		zcrypt_rng_device_count++;
 	mutex_unlock(&zcrypt_rng_mutex);
@@ -1174,6 +1211,10 @@ static void zcrypt_rng_device_remove(voi
 	mutex_lock(&zcrypt_rng_mutex);
 	zcrypt_rng_device_count--;
 	if (zcrypt_rng_device_count == 0) {
+		if (zcrypt_hwrng_fill) {
+			kthread_stop(zcrypt_hwrng_fill);
+			zcrypt_hwrng_fill = 0;
+		}
 		hwrng_unregister(&zcrypt_rng_dev);
 		free_page((unsigned long) zcrypt_rng_buffer);
 	}

Re: [Resend PATCH 2/2] s390: provide hardware randomness from zcrypt card to /dev/random

[H. Peter Anvin]

On 09/12/2013 02:41 AM, Torsten Duwe wrote:
> 
> Running completely virtualised, system Z severely lacks good true random sources.
> Gathering entropy in a virtual environment is difficult. To compensate, there is
> specialised crypto hardware which includes a source for hardware randomness;
> the zcrypt driver is able to access this random source. This patch adds a kernel
> thread that feeds the random bits via the interface created with the previous patch.
> 
> Signed-off-by: Torsten Duwe <duwe@lst.de>

>From what I can gather from the patch this is too heavyweight (need
locks and so on) to use as arch_get_random*().  There has been a lot of
discussion about the pros and cons of allowing the kernel to bypass
rngd, but I would think that any such plumbing -- once it gets past the
fully synchronous low latency properties of arch_get_random*() -- really
should be implemented as an option in the existing hwrng device
infrastructure.

In other words, start by implementing a hwrng device.  That will work
right now with rngd running.  Then we can consider if we want to allow
bypass of rngd for certain hwrng devices -- which may include zcrypt,
virtio_rng and so on.

	-hpa

Re: [Resend PATCH 2/2] s390: provide hardware randomness from zcrypt card to /dev/random

[Torsten Duwe]

On Thu, 12 Sep 2013, H. Peter Anvin wrote:

> From what I can gather from the patch this is too heavyweight (need
> locks and so on) to use as arch_get_random*().  There has been a lot of

Alas, I can see there's only x86 that currently has this implemented?

> discussion about the pros and cons of allowing the kernel to bypass
> rngd, but I would think that any such plumbing -- once it gets past the
> fully synchronous low latency properties of arch_get_random*() -- really
> should be implemented as an option in the existing hwrng device
> infrastructure.

As I wrote in the intro, the problem to solve is slow startup when ASLR is 
in effect; in that case: until rngd or haveged is finally running.

> In other words, start by implementing a hwrng device.  That will work
> right now with rngd running.  Then we can consider if we want to allow

That's already there, thanks to the IBM guys :)

> bypass of rngd for certain hwrng devices -- which may include zcrypt,
> virtio_rng and so on.

I'm currently thinking about some kind of buffer in zcrypt, where 
arch_get_random can get a long or int quickly, as "designed" after x86.
Device init or low water would trigger a work item to refill the buffer.
It might tun out though, that every device on every architecture that does
not quite match the x86 approach implements its own buffer.

What do you think?

Besides that, as you wrote, a generic mechanism to mix hwrngs into the 
input pool would be nice, triggered by user space policy. As far as I can 
see, some mixing of arch_get_random is done, but no entropy credited?

	Torsten

Re: [Resend PATCH 2/2] s390: provide hardware randomness from zcrypt card to /dev/random

[H. Peter Anvin]

By the way, haveged is... worthy of suspicion (I doubt it is malicious, but still).  Its self-tests are completely useless (replace the entropy source with a constant "1" and they still pass) and there is as far as I know no analysis about the randomness other than "it passes some tests."



Torsten Duwe <duwe@lst.de> wrote:
>
>
>On Thu, 12 Sep 2013, H. Peter Anvin wrote:
>
>> From what I can gather from the patch this is too heavyweight (need
>> locks and so on) to use as arch_get_random*().  There has been a lot
>of
>
>Alas, I can see there's only x86 that currently has this implemented?
>
>> discussion about the pros and cons of allowing the kernel to bypass
>> rngd, but I would think that any such plumbing -- once it gets past
>the
>> fully synchronous low latency properties of arch_get_random*() --
>really
>> should be implemented as an option in the existing hwrng device
>> infrastructure.
>
>As I wrote in the intro, the problem to solve is slow startup when ASLR
>is 
>in effect; in that case: until rngd or haveged is finally running.
>
>> In other words, start by implementing a hwrng device.  That will work
>> right now with rngd running.  Then we can consider if we want to
>allow
>
>That's already there, thanks to the IBM guys :)
>
>> bypass of rngd for certain hwrng devices -- which may include zcrypt,
>> virtio_rng and so on.
>
>I'm currently thinking about some kind of buffer in zcrypt, where 
>arch_get_random can get a long or int quickly, as "designed" after x86.
>Device init or low water would trigger a work item to refill the
>buffer.
>It might tun out though, that every device on every architecture that
>does
>not quite match the x86 approach implements its own buffer.
>
>What do you think?
>
>Besides that, as you wrote, a generic mechanism to mix hwrngs into the 
>input pool would be nice, triggered by user space policy. As far as I
>can 
>see, some mixing of arch_get_random is done, but no entropy credited?
>
>	Torsten

-- 
Sent from my mobile phone.  Please pardon brevity and lack of formatting.

Re: [Resend PATCH 2/2] s390: provide hardware randomness from zcrypt card to /dev/random

[H. Peter Anvin]

As I said, the option of doing feed from hwrng directly via a kernel thread seems the most logical thing to me, assuming you can convince Ted & co.  rngd doesn't really add much value for a whitened source.

Torsten Duwe <duwe@lst.de> wrote:
>
>
>On Thu, 12 Sep 2013, H. Peter Anvin wrote:
>
>> From what I can gather from the patch this is too heavyweight (need
>> locks and so on) to use as arch_get_random*().  There has been a lot
>of
>
>Alas, I can see there's only x86 that currently has this implemented?
>
>> discussion about the pros and cons of allowing the kernel to bypass
>> rngd, but I would think that any such plumbing -- once it gets past
>the
>> fully synchronous low latency properties of arch_get_random*() --
>really
>> should be implemented as an option in the existing hwrng device
>> infrastructure.
>
>As I wrote in the intro, the problem to solve is slow startup when ASLR
>is 
>in effect; in that case: until rngd or haveged is finally running.
>
>> In other words, start by implementing a hwrng device.  That will work
>> right now with rngd running.  Then we can consider if we want to
>allow
>
>That's already there, thanks to the IBM guys :)
>
>> bypass of rngd for certain hwrng devices -- which may include zcrypt,
>> virtio_rng and so on.
>
>I'm currently thinking about some kind of buffer in zcrypt, where 
>arch_get_random can get a long or int quickly, as "designed" after x86.
>Device init or low water would trigger a work item to refill the
>buffer.
>It might tun out though, that every device on every architecture that
>does
>not quite match the x86 approach implements its own buffer.
>
>What do you think?
>
>Besides that, as you wrote, a generic mechanism to mix hwrngs into the 
>input pool would be nice, triggered by user space policy. As far as I
>can 
>see, some mixing of arch_get_random is done, but no entropy credited?
>
>	Torsten

-- 
Sent from my mobile phone.  Please pardon brevity and lack of formatting.

[PATCH 00/03]: khwrngd (Was: s390: provide hardware randomness from zcrypt card to /dev/random)

[Torsten Duwe]

On Thu, Sep 19, 2013 at 08:05:32AM -0500, H. Peter Anvin wrote:
> As I said, the option of doing feed from hwrng directly
> via a kernel thread seems the most logical thing to me,
> assuming you can convince Ted & co.
> rngd doesn't really add much value for a whitened source.

Following up on this on (time flies!) I have moved the code
up into the arch-independent hw_random code and split it into 3
patches. First generate the injection point for true HWRNGs,
then create a thread that makes use of it and finally add
some simple on/off switch. The latter could be replaced with
something far more sophisticated if desired.

	Torsten

[Patch 01/03]: provide an injection point for pure hardware randomness

[Torsten Duwe]

This patch adds an interface to the random pool for feeding entropy in-kernel.
It may serve as a destination for dedicated HWRNGs.

It resembles -- and could be merged with -- the ioctl(RNDADDENTROPY) code, plus 
a sleep condition for eager writers.

Signed-off-by: Torsten Duwe <duwe@suse.de>

---
 include/linux/hw_random.h |    2 ++
 drivers/char/random.c     |   23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+)

--- a/include/linux/hw_random.h
+++ b/include/linux/hw_random.h
@@ -47,5 +47,7 @@ struct hwrng {
 extern int hwrng_register(struct hwrng *rng);
 /** Unregister a Hardware Random Number Generator driver. */
 extern void hwrng_unregister(struct hwrng *rng);
+/** Feed random bits into the pool. */
+extern void add_hwgenerator_randomness(const char *buffer, size_t count, size_t entropy);
 
 #endif /* LINUX_HWRANDOM_H_ */
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -250,6 +250,7 @@
 #include <linux/interrupt.h>
 #include <linux/mm.h>
 #include <linux/spinlock.h>
+#include <linux/kthread.h>
 #include <linux/percpu.h>
 #include <linux/cryptohash.h>
 #include <linux/fips.h>
@@ -1347,3 +1347,25 @@ randomize_range(unsigned long start, uns
 		return 0;
 	return PAGE_ALIGN(get_random_int() % range + start);
 }
+
+/* Interface for in-kernel drivers of true hardware RNGs.
+ * Those devices may produce endless random bits and will be throttled
+ * when our pool is full.
+ */
+void add_hwgenerator_randomness(const char *buffer, size_t count,
+				 size_t entropy)
+{
+	struct entropy_store *poolp = &input_pool;
+
+	/* Suspend writing if we're above the trickle threshold.
+	 * We'll be woken up again once below random_write_wakeup_thresh,
+	 * or when the calling thread is about to terminate.
+	 */
+	wait_event_interruptible(random_write_wait, kthread_should_stop() ||
+				 input_pool.entropy_count
+					 <= random_write_wakeup_thresh);
+	mix_pool_bytes(poolp, buffer, count, NULL);
+	credit_entropy_bits(poolp, entropy);
+}
+EXPORT_SYMBOL_GPL(add_hwgenerator_randomness);
+

[PATCH v2 00/03]: khwrngd

[Torsten Duwe]

Here is version 2 of the khwrngd patch set.

The first patch is unchanged.

The second one now introduces an initial derating factor,
as suggested by hpa. It's called derating_current to simplify
patch#3, and the thread creation has moved into hwrng_init,
because it may later depend on the hwrng's derating property.

The third patch only introduces the derating member to
struct hwrng and provides a configurable default.

I could imagine to further add a derating_extra parameter
for conservative admins, in order to diminish the entropy
estimation given from the driver author even more. OTOH
too many knobs might cause confusion.

	Torsten

[PATCH v2 02/03]: hwrng: create filler thread

[Torsten Duwe]

This can be viewed as the in-kernel equivalent of hwrngd;
like FUSE it is a good thing to have a mechanism in user land,
but for some reasons (simplicity, secrecy, integrity, speed)
it may be better to have it in kernel space.

This patch creates a thread once a hwrng registers, and uses
the previously established add_hwgenerator_randomness() to feed
its data to the input pool as long as needed. A derating factor
is used to bias the entropy estimation and to disable this
mechanism entirely when set to zero.

Signed-off-by: Torsten Duwe <duwe@suse.de>

---
 drivers/char/hw_random/core.c |   65 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 62 insertions(+), 3 deletions(-)

--- linux/drivers/char/hw_random/core.c.orig
+++ linux/drivers/char/hw_random/core.c
@@ -39,6 +39,7 @@
 #include <linux/sched.h>
 #include <linux/init.h>
 #include <linux/miscdevice.h>
+#include <linux/kthread.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <asm/uaccess.h>
@@ -50,10 +51,18 @@
 
 
 static struct hwrng *current_rng;
+static struct task_struct *hwrng_fill;
 static LIST_HEAD(rng_list);
 static DEFINE_MUTEX(rng_mutex);
 static int data_avail;
-static u8 *rng_buffer;
+static u8 *rng_buffer, *rng_fillbuf;
+static unsigned short derating_current = 700; /* an arbitrary 70% */
+
+module_param(derating_current, ushort, 0644);
+MODULE_PARM_DESC(derating_current,
+		 "current hwrng entropy estimation per mill");
+
+static void start_khwrngd(void);
 
 static size_t rng_buffer_size(void)
 {
@@ -62,9 +71,18 @@ static size_t rng_buffer_size(void)
 
 static inline int hwrng_init(struct hwrng *rng)
 {
+	int err;
+
 	if (!rng->init)
 		return 0;
-	return rng->init(rng);
+	err = rng->init(rng);
+	if (err)
+		return err;
+
+	if (derating_current > 0 && !hwrng_fill)
+		start_khwrngd();
+
+	return 0;
 }
 
 static inline void hwrng_cleanup(struct hwrng *rng)
@@ -300,6 +318,36 @@ err_misc_dereg:
 	goto out;
 }
 
+static int hwrng_fillfn(void *unused)
+{
+	long rc;
+
+	while (!kthread_should_stop()) {
+		if (!current_rng)
+			break;
+		rc = rng_get_data(current_rng, rng_fillbuf,
+				  rng_buffer_size(), 1);
+		if (rc <= 0) {
+			pr_warn("hwrng: no data available\n");
+			msleep_interruptible(10000);
+			continue;
+		}
+		add_hwgenerator_randomness((void *)rng_fillbuf, rc,
+					   (rc*derating_current)>>10);
+	}
+	hwrng_fill = 0;
+	return 0;
+}
+
+static void start_khwrngd(void)
+{
+	hwrng_fill = kthread_run(hwrng_fillfn, NULL, "hwrng");
+	if (hwrng_fill == ERR_PTR(-ENOMEM)) {
+		pr_err("hwrng_fill thread creation failed");
+		hwrng_fill = NULL;
+	}
+}
+
 int hwrng_register(struct hwrng *rng)
 {
 	int must_register_misc;
@@ -319,6 +367,13 @@ int hwrng_register(struct hwrng *rng)
 		if (!rng_buffer)
 			goto out_unlock;
 	}
+	if (!rng_fillbuf) {
+		rng_fillbuf = kmalloc(rng_buffer_size(), GFP_KERNEL);
+		if (!rng_fillbuf) {
+			kfree(rng_buffer);
+			goto out_unlock;
+		}
+	}
 
 	/* Must not register two RNGs with the same name. */
 	err = -EEXIST;
@@ -373,8 +428,11 @@ void hwrng_unregister(struct hwrng *rng)
 				current_rng = NULL;
 		}
 	}
-	if (list_empty(&rng_list))
+	if (list_empty(&rng_list)) {
 		unregister_miscdev();
+		if (hwrng_fill)
+			kthread_stop(hwrng_fill);
+	}
 
 	mutex_unlock(&rng_mutex);
 }
@@ -385,6 +443,7 @@ static void __exit hwrng_exit(void)
 	mutex_lock(&rng_mutex);
 	BUG_ON(current_rng);
 	kfree(rng_buffer);
+	kfree(rng_fillbuf);
 	mutex_unlock(&rng_mutex);
 }
 

Re: [PATCH v2 02/03]: hwrng: create filler thread

[Andy Lutomirski]

On 03/21/2014 07:33 AM, Torsten Duwe wrote:
> This can be viewed as the in-kernel equivalent of hwrngd;
> like FUSE it is a good thing to have a mechanism in user land,
> but for some reasons (simplicity, secrecy, integrity, speed)
> it may be better to have it in kernel space.

Nice.


[...]

>  
>  static struct hwrng *current_rng;
> +static struct task_struct *hwrng_fill;
>  static LIST_HEAD(rng_list);
>  static DEFINE_MUTEX(rng_mutex);
>  static int data_avail;
> -static u8 *rng_buffer;
> +static u8 *rng_buffer, *rng_fillbuf;
> +static unsigned short derating_current = 700; /* an arbitrary 70% */
> +
> +module_param(derating_current, ushort, 0644);
> +MODULE_PARM_DESC(derating_current,
> +		 "current hwrng entropy estimation per mill");

As an electrical engineer (sort of), I can't read this without thinking
you're talking about the amount by which the current is derated.  For
example, a 14-50 electrical outlet is rated to 50 Amps.  If you use it
continuously for a long time, though, the current is derated to 40 Amps.

Shouldn't this be called credit_derating or, even better,
credit_per_1000bits?

Also, "per mill" is just obscure enough that someone might think it
means "per million".


> +
> +static void start_khwrngd(void);
>  
>  static size_t rng_buffer_size(void)
>  {
> @@ -62,9 +71,18 @@ static size_t rng_buffer_size(void)
>  
>  static inline int hwrng_init(struct hwrng *rng)
>  {
> +	int err;
> +
>  	if (!rng->init)
>  		return 0;
> -	return rng->init(rng);
> +	err = rng->init(rng);
> +	if (err)
> +		return err;
> +
> +	if (derating_current > 0 && !hwrng_fill)
> +		start_khwrngd();
> +

Why the check for derating > 0?  Paranoid users may want zero credit,
but they probably still want the thing to run.

> +	return 0;
>  }
>  
>  static inline void hwrng_cleanup(struct hwrng *rng)
> @@ -300,6 +318,36 @@ err_misc_dereg:
>  	goto out;
>  }
>  
> +static int hwrng_fillfn(void *unused)
> +{
> +	long rc;
> +
> +	while (!kthread_should_stop()) {
> +		if (!current_rng)
> +			break;
> +		rc = rng_get_data(current_rng, rng_fillbuf,
> +				  rng_buffer_size(), 1);
> +		if (rc <= 0) {
> +			pr_warn("hwrng: no data available\n");

ratelimit (heavily), please.

Also, would it make sense to round-robin all hwrngs?  Even better:
collect entropy from each one and add them to the pool all at once.  If
so, would it make sense for the derating to be a per-rng parameter.  For
example, if there's a sysfs class, it could go in there.

Finally, there may be hwrngs like TPMs that are amazingly slow.  What
happens if the RNG is so slow that it becomes the bottleneck?  Should
this thing back off?  Using the TPM at 100% utilization seems silly when
there's a heavy entropy consumer, especially since reading 256 bits from
the TPM once is probably just about as secure as reading from it
continuously.


Also, with my quantum hat on, thanks for doing this in a way that isn't
gratuitously insecure against quantum attack.  128-bit reseeds are
simply too small if your adversary has a large quantum computer :)


--Andy

Re: [PATCH v2 02/03]: hwrng: create filler thread

[H. Peter Anvin]

I'm wondering more about the default.  We default to 50% for arch_get_random_seed, and this is supposed to be the default for in effect unverified hwrngs...

On March 26, 2014 5:50:09 PM PDT, Andy Lutomirski <luto@amacapital.net> wrote:
>On 03/21/2014 07:33 AM, Torsten Duwe wrote:
>> This can be viewed as the in-kernel equivalent of hwrngd;
>> like FUSE it is a good thing to have a mechanism in user land,
>> but for some reasons (simplicity, secrecy, integrity, speed)
>> it may be better to have it in kernel space.
>
>Nice.
>
>
>[...]
>
>>  
>>  static struct hwrng *current_rng;
>> +static struct task_struct *hwrng_fill;
>>  static LIST_HEAD(rng_list);
>>  static DEFINE_MUTEX(rng_mutex);
>>  static int data_avail;
>> -static u8 *rng_buffer;
>> +static u8 *rng_buffer, *rng_fillbuf;
>> +static unsigned short derating_current = 700; /* an arbitrary 70% */
>> +
>> +module_param(derating_current, ushort, 0644);
>> +MODULE_PARM_DESC(derating_current,
>> +		 "current hwrng entropy estimation per mill");
>
>As an electrical engineer (sort of), I can't read this without thinking
>you're talking about the amount by which the current is derated.  For
>example, a 14-50 electrical outlet is rated to 50 Amps.  If you use it
>continuously for a long time, though, the current is derated to 40
>Amps.
>
>Shouldn't this be called credit_derating or, even better,
>credit_per_1000bits?
>
>Also, "per mill" is just obscure enough that someone might think it
>means "per million".
>
>
>> +
>> +static void start_khwrngd(void);
>>  
>>  static size_t rng_buffer_size(void)
>>  {
>> @@ -62,9 +71,18 @@ static size_t rng_buffer_size(void)
>>  
>>  static inline int hwrng_init(struct hwrng *rng)
>>  {
>> +	int err;
>> +
>>  	if (!rng->init)
>>  		return 0;
>> -	return rng->init(rng);
>> +	err = rng->init(rng);
>> +	if (err)
>> +		return err;
>> +
>> +	if (derating_current > 0 && !hwrng_fill)
>> +		start_khwrngd();
>> +
>
>Why the check for derating > 0?  Paranoid users may want zero credit,
>but they probably still want the thing to run.
>
>> +	return 0;
>>  }
>>  
>>  static inline void hwrng_cleanup(struct hwrng *rng)
>> @@ -300,6 +318,36 @@ err_misc_dereg:
>>  	goto out;
>>  }
>>  
>> +static int hwrng_fillfn(void *unused)
>> +{
>> +	long rc;
>> +
>> +	while (!kthread_should_stop()) {
>> +		if (!current_rng)
>> +			break;
>> +		rc = rng_get_data(current_rng, rng_fillbuf,
>> +				  rng_buffer_size(), 1);
>> +		if (rc <= 0) {
>> +			pr_warn("hwrng: no data available\n");
>
>ratelimit (heavily), please.
>
>Also, would it make sense to round-robin all hwrngs?  Even better:
>collect entropy from each one and add them to the pool all at once.  If
>so, would it make sense for the derating to be a per-rng parameter. 
>For
>example, if there's a sysfs class, it could go in there.
>
>Finally, there may be hwrngs like TPMs that are amazingly slow.  What
>happens if the RNG is so slow that it becomes the bottleneck?  Should
>this thing back off?  Using the TPM at 100% utilization seems silly
>when
>there's a heavy entropy consumer, especially since reading 256 bits
>from
>the TPM once is probably just about as secure as reading from it
>continuously.
>
>
>Also, with my quantum hat on, thanks for doing this in a way that isn't
>gratuitously insecure against quantum attack.  128-bit reseeds are
>simply too small if your adversary has a large quantum computer :)
>
>
>--Andy

-- 
Sent from my mobile phone.  Please pardon brevity and lack of formatting.

[PATCH v3 00/03]: hwrng: an in-kernel rngd

[Torsten Duwe]

More or less a resend of v2.

On Wed, Mar 26, 2014 at 06:03:37PM -0700, H. Peter Anvin wrote:
> I'm wondering more about the default.  We default to 50% for arch_get_random_seed, and this is supposed to be the default for in effect unverified hwrngs...

Done. 50% is now the default, that's the only change from v2.

Andy: the printk you pointed out already limits itself to 1/10s,
which is half the default rate limit. Also, as Peter already
wrote, we're dealing with true HWRNGs here; if such a device
does not produce a single byte within 10 seconds something _is_
severely broken and, like a dying disk, worth to be logged.
Here's one of the better circuits I found:
http://www.maximintegrated.com/app-notes/index.mvp/id/3469
or offline:
http://pdfserv.maximintegrated.com/en/an/AN3469.pdf
Disclaimer: I'm not endorsing Maxim, it's just that paper
that hits the spot IMHO.

Anything wrong with feeding those bits into the input pool?
Any other comments on the code?

	Torsten

[PATCH v3 03/03]: hwrng: khwrngd derating per device

[Torsten Duwe]

This patch introduces a derating factor to struct hwrng for
the random bits going into the kernel input pool, and a common
default derating for drivers which do not specify one.

Signed-off-by: Torsten Duwe <duwe@suse.de>

---
 drivers/char/hw_random/core.c |   11 ++++++++++-
 include/linux/hw_random.h     |    3 +++
 2 files changed, 13 insertions(+), 1 deletion(-)

--- linux/include/linux/hw_random.h.orig
+++ linux/include/linux/hw_random.h
@@ -29,6 +29,8 @@
  * @read:		New API. drivers can fill up to max bytes of data
  *			into the buffer. The buffer is aligned for any type.
  * @priv:		Private data, for use by the RNG driver.
+ * @derating:		Estimation of true entropy in RNG's bitstream
+ *			(per mill).
  */
 struct hwrng {
 	const char *name;
@@ -38,6 +39,7 @@ struct hwrng {
 	int (*data_read)(struct hwrng *rng, u32 *data);
 	int (*read)(struct hwrng *rng, void *data, size_t max, bool wait);
 	unsigned long priv;
+	unsigned short derating;
 
 	/* internal. */
 	struct list_head list;
--- linux/drivers/char/hw_random/core.c.orig
+++ linux/drivers/char/hw_random/core.c
@@ -56,11 +56,15 @@ static LIST_HEAD(rng_list);
 static DEFINE_MUTEX(rng_mutex);
 static int data_avail;
 static u8 *rng_buffer, *rng_fillbuf;
-static unsigned short derating_current = 700; /* an arbitrary 70% */
+static unsigned short derating_current;
+static unsigned short derating_default = 500; /* an arbitrary 50% */
 
 module_param(derating_current, ushort, 0644);
 MODULE_PARM_DESC(derating_current,
 		 "current hwrng entropy estimation per mill");
+module_param(derating_default, ushort, 0644);
+MODULE_PARM_DESC(derating_default,
+		 "default entropy content of hwrng per mill");
 
 static void start_khwrngd(void);
 
@@ -79,6 +83,11 @@ static inline int hwrng_init(struct hwrn
 	if (err)
 		return err;
 
+	derating_current = rng->derating ? : derating_default;
+	derating_current &= 1023;
+
+	if (derating_current == 0 && hwrng_fill)
+		kthread_stop(hwrng_fill);
 	if (derating_current > 0 && !hwrng_fill)
 		start_khwrngd();
 

Re: [PATCH v3 03/03]: hwrng: khwrngd derating per device

[Andy Lutomirski]

On Mon, Apr 14, 2014 at 9:06 AM, Torsten Duwe <duwe@lst.de> wrote:
>
> This patch introduces a derating factor to struct hwrng for
> the random bits going into the kernel input pool, and a common
> default derating for drivers which do not specify one.
>
> Signed-off-by: Torsten Duwe <duwe@suse.de>
>
> ---
>  drivers/char/hw_random/core.c |   11 ++++++++++-
>  include/linux/hw_random.h     |    3 +++
>  2 files changed, 13 insertions(+), 1 deletion(-)
>
> --- linux/include/linux/hw_random.h.orig
> +++ linux/include/linux/hw_random.h
> @@ -29,6 +29,8 @@
>   * @read:              New API. drivers can fill up to max bytes of data
>   *                     into the buffer. The buffer is aligned for any type.
>   * @priv:              Private data, for use by the RNG driver.
> + * @derating:          Estimation of true entropy in RNG's bitstream
> + *                     (per mill).

I'll bikeshed again: this is a rating, not a *de*rating.  Higher =
more confidence, at least assuming the comment is right.

--Andy

Re: [PATCH v3 03/03]: hwrng: khwrngd derating per device

[Torsten Duwe]

On Mon, Apr 14, 2014 at 09:41:10AM -0700, Andy Lutomirski wrote:
> On Mon, Apr 14, 2014 at 9:06 AM, Torsten Duwe <duwe@lst.de> wrote:
> >
> > This patch introduces a derating factor to struct hwrng for
> > the random bits going into the kernel input pool, and a common
> > default derating for drivers which do not specify one.
> >
> > Signed-off-by: Torsten Duwe <duwe@suse.de>
> >
> > ---
> >  drivers/char/hw_random/core.c |   11 ++++++++++-
> >  include/linux/hw_random.h     |    3 +++
> >  2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > --- linux/include/linux/hw_random.h.orig
> > +++ linux/include/linux/hw_random.h
> > @@ -29,6 +29,8 @@
> >   * @read:              New API. drivers can fill up to max bytes of data
> >   *                     into the buffer. The buffer is aligned for any type.
> >   * @priv:              Private data, for use by the RNG driver.
> > + * @derating:          Estimation of true entropy in RNG's bitstream
> > + *                     (per mill).
> 
> I'll bikeshed again: this is a rating, not a *de*rating.  Higher =
> more confidence, at least assuming the comment is right.
> 
You're right. Would anyone object to call it "quality", as in RX signal quality?
In context of a random source that is pretty accurate, I'd say. Other opinions?

	Torsten

Re: [PATCH v3 03/03]: hwrng: khwrngd derating per device

[Andy Lutomirski]

On Tue, Apr 15, 2014 at 1:51 AM, Torsten Duwe <duwe@lst.de> wrote:
> On Mon, Apr 14, 2014 at 09:41:10AM -0700, Andy Lutomirski wrote:
>> On Mon, Apr 14, 2014 at 9:06 AM, Torsten Duwe <duwe@lst.de> wrote:
>> >
>> > This patch introduces a derating factor to struct hwrng for
>> > the random bits going into the kernel input pool, and a common
>> > default derating for drivers which do not specify one.
>> >
>> > Signed-off-by: Torsten Duwe <duwe@suse.de>
>> >
>> > ---
>> >  drivers/char/hw_random/core.c |   11 ++++++++++-
>> >  include/linux/hw_random.h     |    3 +++
>> >  2 files changed, 13 insertions(+), 1 deletion(-)
>> >
>> > --- linux/include/linux/hw_random.h.orig
>> > +++ linux/include/linux/hw_random.h
>> > @@ -29,6 +29,8 @@
>> >   * @read:              New API. drivers can fill up to max bytes of data
>> >   *                     into the buffer. The buffer is aligned for any type.
>> >   * @priv:              Private data, for use by the RNG driver.
>> > + * @derating:          Estimation of true entropy in RNG's bitstream
>> > + *                     (per mill).
>>
>> I'll bikeshed again: this is a rating, not a *de*rating.  Higher =
>> more confidence, at least assuming the comment is right.
>>
> You're right. Would anyone object to call it "quality", as in RX signal quality?
> In context of a random source that is pretty accurate, I'd say. Other opinions?

I'm okay with "quality", although I'm still partial to "entropy_per_1000bits".

--Andy

[PATCH v5 00/03]: hwrng: an in-kernel rngd

[Torsten Duwe]

On Tue, Apr 15, 2014 at 09:53:53AM -0700, Andy Lutomirski wrote:
> On Tue, Apr 15, 2014 at 1:51 AM, Torsten Duwe <duwe@lst.de> wrote:
> > You're right. Would anyone object to call it "quality", as in RX signal quality?
> > In context of a random source that is pretty accurate, I'd say. Other opinions?
> 
> I'm okay with "quality", although I'm still partial to "entropy_per_1000bits".

So, since there were no other comments, here's v5 with aforementioned change.
Default is still "off". Patch 02/03 de-fuzzed for 3.15.

	Torsten

[Patch 01/03]: provide an injection point for pure hardware randomness

[Torsten Duwe]

This patch adds an interface to the random pool for feeding entropy in-kernel.
It may serve as a destination for dedicated HWRNGs.

It resembles -- and could be merged with -- the ioctl(RNDADDENTROPY) code, plus 
a sleep condition for eager writers.

Signed-off-by: Torsten Duwe <duwe@suse.de>

---
 include/linux/hw_random.h |    2 ++
 drivers/char/random.c     |   23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+)

--- a/include/linux/hw_random.h
+++ b/include/linux/hw_random.h
@@ -47,5 +47,7 @@ struct hwrng {
 extern int hwrng_register(struct hwrng *rng);
 /** Unregister a Hardware Random Number Generator driver. */
 extern void hwrng_unregister(struct hwrng *rng);
+/** Feed random bits into the pool. */
+extern void add_hwgenerator_randomness(const char *buffer, size_t count, size_t entropy);
 
 #endif /* LINUX_HWRANDOM_H_ */
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -250,6 +250,7 @@
 #include <linux/interrupt.h>
 #include <linux/mm.h>
 #include <linux/spinlock.h>
+#include <linux/kthread.h>
 #include <linux/percpu.h>
 #include <linux/cryptohash.h>
 #include <linux/fips.h>
@@ -1347,3 +1347,25 @@ randomize_range(unsigned long start, uns
 		return 0;
 	return PAGE_ALIGN(get_random_int() % range + start);
 }
+
+/* Interface for in-kernel drivers of true hardware RNGs.
+ * Those devices may produce endless random bits and will be throttled
+ * when our pool is full.
+ */
+void add_hwgenerator_randomness(const char *buffer, size_t count,
+				 size_t entropy)
+{
+	struct entropy_store *poolp = &input_pool;
+
+	/* Suspend writing if we're above the trickle threshold.
+	 * We'll be woken up again once below random_write_wakeup_thresh,
+	 * or when the calling thread is about to terminate.
+	 */
+	wait_event_interruptible(random_write_wait, kthread_should_stop() ||
+				 input_pool.entropy_count
+					 <= random_write_wakeup_thresh);
+	mix_pool_bytes(poolp, buffer, count, NULL);
+	credit_entropy_bits(poolp, entropy);
+}
+EXPORT_SYMBOL_GPL(add_hwgenerator_randomness);
+