[PATCH v2 00/03]: khwrngd

[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 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 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);
 }
 

[PATCH v2 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 = 700; /* an arbitrary 70% */
 
 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 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.

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

[Andy Lutomirski]

[cc: Greg Price, might be working on this stuff]

On Wed, Mar 26, 2014 at 6:03 PM, H. Peter Anvin <hpa@zytor.com> 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...

TBH I'm highly skeptical of this kind of entropy estimation.
/dev/random is IMO just silly, since you need to have very
conservative entropy estimates for the concept to really work, and
that ends up being hideously slow.  Also, in the /dev/random sense,
most hardware RNGs have no entropy at all, since they're likely to be
FIPS-approved DRBGs that don't have a real non-deterministic source.

For the kernel's RNG to be secure, I think it should have the property
that it still works if you rescale all the entropy estimates by any
constant that's decently close to 1.

If entropy estimates are systematically too low, then a naive
implementation results in an excessively long window during early
bootup in which /dev/urandom is completely insecure.

If entropy estimates are systematically too high, then a naive
implementation fails to do a catastrophic reseed, and the RNG can be
brute-forced.

So I think that the core code should do something along the lines of
using progressively larger reseeds.  Since I think that /dev/random is
silly, this means that we only really care about the extent to which
"entropy" measures entropy conditioned on whatever an attacker can
actually compute.  Since this could vary widely between devices (e.g.
if your TPM is malicious), I think that the best we can do is to
collect ~256 bits from everything available, shove it all in to the
core together, and repeat.  For all I know, the core code already does
this.

The upshot is that the actual rescaling factor should barely matter.
50% is probably fine.  So is 100% and 25%.  10% is probably asking for
trouble during early boot if all you have is a TPM.

--Andy

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

[H. Peter Anvin]

There are a number of things wrong with this post, but I'll respond in detail when I get to a keyboard.

On March 26, 2014 6:11:53 PM PDT, Andy Lutomirski <luto@amacapital.net> wrote:
>[cc: Greg Price, might be working on this stuff]
>
>On Wed, Mar 26, 2014 at 6:03 PM, H. Peter Anvin <hpa@zytor.com> 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...
>
>TBH I'm highly skeptical of this kind of entropy estimation.
>/dev/random is IMO just silly, since you need to have very
>conservative entropy estimates for the concept to really work, and
>that ends up being hideously slow.  Also, in the /dev/random sense,
>most hardware RNGs have no entropy at all, since they're likely to be
>FIPS-approved DRBGs that don't have a real non-deterministic source.
>
>For the kernel's RNG to be secure, I think it should have the property
>that it still works if you rescale all the entropy estimates by any
>constant that's decently close to 1.
>
>If entropy estimates are systematically too low, then a naive
>implementation results in an excessively long window during early
>bootup in which /dev/urandom is completely insecure.
>
>If entropy estimates are systematically too high, then a naive
>implementation fails to do a catastrophic reseed, and the RNG can be
>brute-forced.
>
>So I think that the core code should do something along the lines of
>using progressively larger reseeds.  Since I think that /dev/random is
>silly, this means that we only really care about the extent to which
>"entropy" measures entropy conditioned on whatever an attacker can
>actually compute.  Since this could vary widely between devices (e.g.
>if your TPM is malicious), I think that the best we can do is to
>collect ~256 bits from everything available, shove it all in to the
>core together, and repeat.  For all I know, the core code already does
>this.
>
>The upshot is that the actual rescaling factor should barely matter.
>50% is probably fine.  So is 100% and 25%.  10% is probably asking for
>trouble during early boot if all you have is a TPM.
>
>--Andy

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

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

[H. Peter Anvin]

On 03/26/2014 06:11 PM, Andy Lutomirski wrote:
> 
> TBH I'm highly skeptical of this kind of entropy estimation.
> /dev/random is IMO just silly, since you need to have very
> conservative entropy estimates for the concept to really work, and
> that ends up being hideously slow.

In the absence of a hardware entropy source, it is, but for long-lived
keys, delay is better than bad key generation.

A major reason for entropy estimation is to control the amount of
backpressure.  If you don't have backpressure, you only have generation
pressure, and you can't put your system to sleep when the hwrng keeps
outputting data.  Worse, if your entropy source is inexhaustible, you
might end up spending all your CPU time processing its output.

> Also, in the /dev/random sense,
> most hardware RNGs have no entropy at all, since they're likely to be
> FIPS-approved DRBGs that don't have a real non-deterministic source.

Such a device has no business being a Linux hwrng device.  We already
have a PRNG (DRBG) in the kernel, the *only* purpose for a hwrng device
is to be an entropy source.

> For the kernel's RNG to be secure, I think it should have the property
> that it still works if you rescale all the entropy estimates by any
> constant that's decently close to 1.

That is correct.

> If entropy estimates are systematically too low, then a naive
> implementation results in an excessively long window during early
> bootup in which /dev/urandom is completely insecure.

Eh?  What mechanism would make /dev/urandom any less secure due to
entropy underestimation?  The whole *point* is that we should
systematically underestimate entropy -- and we do, according to research
papers which have analyzed the state of things we do by orders of
magnitude, which is the only possible way to do it for non-hwrng sources.

> If entropy estimates are systematically too high, then a naive
> implementation fails to do a catastrophic reseed, and the RNG can be
> brute-forced.

This again is unacceptable.  We really should not overestimate.

> So I think that the core code should do something along the lines of
> using progressively larger reseeds.  Since I think that /dev/random is
> silly, this means that we only really care about the extent to which
> "entropy" measures entropy conditioned on whatever an attacker can
> actually compute.  Since this could vary widely between devices (e.g.
> if your TPM is malicious), I think that the best we can do is to
> collect ~256 bits from everything available, shove it all in to the
> core together, and repeat.  For all I know, the core code already does
> this.
> 
> The upshot is that the actual rescaling factor should barely matter.
> 50% is probably fine.  So is 100% and 25%.  10% is probably asking for
> trouble during early boot if all you have is a TPM.

I don't see why small factors should be a problem at all (except that it
discourages /dev/random usage.)  Keep in mind we still add the entropy
-- we just don't credit its existence.

TPMs, in particular, should almost certainly be massively derated based
on what little we know about TPM.

As a concrete example: RDRAND is a hardware entropy source that is
architecturally allowed to be diluted by a DRBG up to 512 times.  As far
as I know of the hardware, no shipping piece of hardware is anywhere
near 512 in this aspect.  rngd currently does 512:1 data reduction, but
injecting the raw output at 1/512 credit ought to give a much better
result in terms of entropy.

	-hpa

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

[Torsten Duwe]

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...

If the default were 0, it would be exactly the old behaviour.
How about that? Plus, driver authors would have to come up
with an estimate on their own.

> On March 26, 2014 5:50:09 PM PDT, Andy Lutomirski <luto@amacapital.net> wrote:
> >> +		 "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?

That's an awkward name for a parameter.

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

No. I looked it up, as we have the precise term "Promille" in German.
Also in electrical engineering, (imperial :-) PCB design, a mil
is one 1000th of an inch. Per million would surely be named PPM.

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

[...]

> >ratelimit (heavily), please.

The kthread will stop once the estimated entropy is above the threshold.
derating=0 will wind up one CPU core to 100%. So it's an elegant way
to disable the whole mechanism.

> >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. 

Finally, the derating _is_ a per-RNG parameter. I also thought about 
mixing already, but first I want to see a machine with more than 1 HWRNG :-)

	Torsten

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

[Torsten Duwe]

On Wed, Mar 26, 2014 at 09:47:31PM -0700, H. Peter Anvin wrote:
> On 03/26/2014 06:11 PM, Andy Lutomirski wrote:
> > Also, in the /dev/random sense,
> > most hardware RNGs have no entropy at all, since they're likely to be
> > FIPS-approved DRBGs that don't have a real non-deterministic source.
> 
> Such a device has no business being a Linux hwrng device.  We already
> have a PRNG (DRBG) in the kernel, the *only* purpose for a hwrng device
> is to be an entropy source.

That's also my understanding. The shot noise from a Schottky-diode seems
to be quite popular, for example.

> TPMs, in particular, should almost certainly be massively derated based
> on what little we know about TPM.

Backdoors, anyone? :-)

	Torsten

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

[Andy Lutomirski]

On Thu, Mar 27, 2014 at 7:54 AM, Torsten Duwe <duwe@lst.de> wrote:
> 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...
>
> If the default were 0, it would be exactly the old behaviour.
> How about that? Plus, driver authors would have to come up
> with an estimate on their own.
>
>> On March 26, 2014 5:50:09 PM PDT, Andy Lutomirski <luto@amacapital.net> wrote:
>> >> +           "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?
>
> That's an awkward name for a parameter.

I don't think it's worse than credit_derating.

>
>> >Also, "per mill" is just obscure enough that someone might think it
>> >means "per million".
>
> No. I looked it up, as we have the precise term "Promille" in German.
> Also in electrical engineering, (imperial :-) PCB design, a mil
> is one 1000th of an inch. Per million would surely be named PPM.
>

I'm not saying that "per mill" is wrong -- I'm just saying it's
obscure and may confuse people.

>> >Why the check for derating > 0?  Paranoid users may want zero credit,
>> >but they probably still want the thing to run.
>
> [...]
>
>> >ratelimit (heavily), please.
>
> The kthread will stop once the estimated entropy is above the threshold.
> derating=0 will wind up one CPU core to 100%. So it's an elegant way
> to disable the whole mechanism.
>

Sorry, I didn't mean ratelimit the loop.  I meant ratelimit the printk.

>> >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.
>
> Finally, the derating _is_ a per-RNG parameter. I also thought about
> mixing already, but first I want to see a machine with more than 1 HWRNG :-)
>

Any Haswell machine with another hwrng will have two of them.  (I'm
not sure that the rdrand/rdseed thing registers as an hwrng, but
still...)

--Andy

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

[Andy Lutomirski]

On Wed, Mar 26, 2014 at 9:47 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 03/26/2014 06:11 PM, Andy Lutomirski wrote:
>>
>> TBH I'm highly skeptical of this kind of entropy estimation.
>> /dev/random is IMO just silly, since you need to have very
>> conservative entropy estimates for the concept to really work, and
>> that ends up being hideously slow.
>
> In the absence of a hardware entropy source, it is, but for long-lived
> keys, delay is better than bad key generation.
>
> A major reason for entropy estimation is to control the amount of
> backpressure.  If you don't have backpressure, you only have generation
> pressure, and you can't put your system to sleep when the hwrng keeps
> outputting data.  Worse, if your entropy source is inexhaustible, you
> might end up spending all your CPU time processing its output.

Fair enough.  I'll shut up about /dev/random (except for the fact that
I think that the reseed logic should be considered very carefully).
Please consider the rest of my comments as being specific to urandom.
:)

>
>> Also, in the /dev/random sense,
>> most hardware RNGs have no entropy at all, since they're likely to be
>> FIPS-approved DRBGs that don't have a real non-deterministic source.
>
> Such a device has no business being a Linux hwrng device.  We already
> have a PRNG (DRBG) in the kernel, the *only* purpose for a hwrng device
> is to be an entropy source.

See the very end.

>
>> For the kernel's RNG to be secure, I think it should have the property
>> that it still works if you rescale all the entropy estimates by any
>> constant that's decently close to 1.
>
> That is correct.
>
>> If entropy estimates are systematically too low, then a naive
>> implementation results in an excessively long window during early
>> bootup in which /dev/urandom is completely insecure.
>
> Eh?  What mechanism would make /dev/urandom any less secure due to
> entropy underestimation?  The whole *point* is that we should
> systematically underestimate entropy -- and we do, according to research
> papers which have analyzed the state of things we do by orders of
> magnitude, which is the only possible way to do it for non-hwrng sources.

Lack of an initial reseed.  If the core code decides that it's only
received three bits of entropy and shouldn't reseed, then the system
might go for a very long time with no entropy at all making it all the
way to urandom.  I don't know what the current code does, but it's
changed quite a few times recently.

>
>> If entropy estimates are systematically too high, then a naive
>> implementation fails to do a catastrophic reseed, and the RNG can be
>> brute-forced.
>
> This again is unacceptable.  We really should not overestimate.

I think we shouldn't overestimate, but I think that we should also
have an implementation that's robust against overestimating by a
moderate factor.

>
>> So I think that the core code should do something along the lines of
>> using progressively larger reseeds.  Since I think that /dev/random is
>> silly, this means that we only really care about the extent to which
>> "entropy" measures entropy conditioned on whatever an attacker can
>> actually compute.  Since this could vary widely between devices (e.g.
>> if your TPM is malicious), I think that the best we can do is to
>> collect ~256 bits from everything available, shove it all in to the
>> core together, and repeat.  For all I know, the core code already does
>> this.
>>
>> The upshot is that the actual rescaling factor should barely matter.
>> 50% is probably fine.  So is 100% and 25%.  10% is probably asking for
>> trouble during early boot if all you have is a TPM.
>
> I don't see why small factors should be a problem at all (except that it
> discourages /dev/random usage.)  Keep in mind we still add the entropy
> -- we just don't credit its existence.
>

This is only true if the entropy actually makes it to /dev/urandom.
If the input pool credited entropy is too small, then account, and
hence extract_entropy, will return zero when called on the input pool,
xfer_secondary_pool on the urandom pool won't do anything, so urandom
won't be reseeded at all.

Damn it, this is crypto code.  It should not be this hard to
understand what the code is doing.

> TPMs, in particular, should almost certainly be massively derated based
> on what little we know about TPM.
>
> As a concrete example: RDRAND is a hardware entropy source that is
> architecturally allowed to be diluted by a DRBG up to 512 times.  As far
> as I know of the hardware, no shipping piece of hardware is anywhere
> near 512 in this aspect.  rngd currently does 512:1 data reduction, but
> injecting the raw output at 1/512 credit ought to give a much better
> result in terms of entropy.

Hmm.  Maybe the core random code should have a separate way to inject
cryptographic entropy-less bits.  I agree that the TPM has no business
providing any credit at all to /dev/random, but I think that it would
be a huge improvement to use the TPM at least on startup to seed
urandom.  It's there and, however weak it may be, it's a lot better
than not seeding urandom at all.

This could be as simple as add_drbg_randomness.  It doesn't need to go
through hwrng.

rdrand is weird and I have no real problem with sticking it in to
/dev/random with a suitable derating.  It's a DRBG, but it's also
seeded with a read RNG, whereas I suspect that most TPMs have no real
entropy source, or at least no entropy source fast enough to be useful
if the TPM's crypto is bad.

--Andy

[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 01/03]: hwrng: 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 v3 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);
 }
 

[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 00/03]: hwrng: an in-kernel rngd

[H. Peter Anvin]

I think the default should be zero, so each hwrng driver maintainer would have to consider what guarantees that particular driver can give.  If anything 50% ought to be the maximum.

On April 14, 2014 9:02:11 AM PDT, Torsten Duwe <duwe@lst.de> wrote:
>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

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

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

[Torsten Duwe]

On Mon, Apr 14, 2014 at 09:09:14AM -0700, H. Peter Anvin wrote:
> I think the default should be zero, so each hwrng driver maintainer would have to consider what guarantees that particular driver can give.  If anything 50% ought to be the maximum.

Repost of v4 just for patch 03/03 is on its way then, defaulting to 0
(off), which happens to be the old behaviour, which I think I've also
expressed my sympathy for already.

	Torsten

[PATCH v4 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 = 0; /* default to "off" */
 
 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 00/03]: hwrng: an in-kernel rngd

[H. Peter Anvin]

On 04/14/2014 09:24 AM, Torsten Duwe wrote:
> On Mon, Apr 14, 2014 at 09:09:14AM -0700, H. Peter Anvin wrote:
>> I think the default should be zero, so each hwrng driver maintainer would have to consider what guarantees that particular driver can give.  If anything 50% ought to be the maximum.
> 
> Repost of v4 just for patch 03/03 is on its way then, defaulting to 0
> (off), which happens to be the old behaviour, which I think I've also
> expressed my sympathy for already.
> 

Sounds good.

	-hpa

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 00/03]: hwrng: an in-kernel rngd

[Andy Lutomirski]

On Mon, Apr 14, 2014 at 9:24 AM, Torsten Duwe <duwe@lst.de> wrote:
> On Mon, Apr 14, 2014 at 09:09:14AM -0700, H. Peter Anvin wrote:
>> I think the default should be zero, so each hwrng driver maintainer would have to consider what guarantees that particular driver can give.  If anything 50% ought to be the maximum.
>
> Repost of v4 just for patch 03/03 is on its way then, defaulting to 0
> (off), which happens to be the old behaviour, which I think I've also
> expressed my sympathy for already.
>

To be clear, I like your patches, bikeshedding aside.  Having reads of
/dev/random automatically pull in hwrng data would be *great*.  My
drbg patches are not meant to replace them at all.  I just think that
there's a need for the nonblocking pool to be well-seeded as quickly
as possible, regardless of configuration, and I don't think your code
is meant to do that.

--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);
+

[Patch v5 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/fs.h>
 #include <linux/sched.h>
 #include <linux/miscdevice.h>
+#include <linux/kthread.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/random.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 current_quality = 700; /* an arbitrary 70% */
+
+module_param(current_quality, ushort, 0644);
+MODULE_PARM_DESC(current_quality,
+		 "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 (current_quality > 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*current_quality)>>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 err = -EINVAL;
@@ -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);
 }
 

[Patch v5 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.
+ * @quality:		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 quality;
 
 	/* 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 current_quality = 700; /* an arbitrary 70% */
+static unsigned short current_quality;
+static unsigned short default_quality = 0; /* default to "off" */
 
 module_param(current_quality, ushort, 0644);
 MODULE_PARM_DESC(current_quality,
 		 "current hwrng entropy estimation per mill");
+module_param(default_quality, ushort, 0644);
+MODULE_PARM_DESC(default_quality,
+		 "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;
 
+	current_quality = rng->quality ? : default_quality;
+	current_quality &= 1023;
+
+	if (current_quality == 0 && hwrng_fill)
+		kthread_stop(hwrng_fill);
 	if (current_quality > 0 && !hwrng_fill)
 		start_khwrngd();
 

[Patch v5.1 03/03]: hwrng: khwrngd derating per device

[Torsten Duwe]

[checkpatch tells me not to 0-init...]

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.
+ * @quality:		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 quality;
 
 	/* 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 current_quality = 700; /* an arbitrary 70% */
+static unsigned short current_quality;
+static unsigned short default_quality; /* = 0; default to "off" */
 
 module_param(current_quality, ushort, 0644);
 MODULE_PARM_DESC(current_quality,
 		 "current hwrng entropy estimation per mill");
+module_param(default_quality, ushort, 0644);
+MODULE_PARM_DESC(default_quality,
+		 "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;
 
+	current_quality = rng->quality ? : default_quality;
+	current_quality &= 1023;
+
+	if (current_quality == 0 && hwrng_fill)
+		kthread_stop(hwrng_fill);
 	if (current_quality > 0 && !hwrng_fill)
 		start_khwrngd();
 

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

[H. Peter Anvin]

On 05/27/2014 07:11 AM, Torsten Duwe wrote:
> [checkpatch tells me not to 0-init...]
> 
> 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>
> 

Did we lose track of this patchset?

	-hpa

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

[Torsten Duwe]

On Wed, Jun 11, 2014 at 06:24:53PM -0700, H. Peter Anvin wrote:
> On 05/27/2014 07:11 AM, Torsten Duwe wrote:
> > [checkpatch tells me not to 0-init...]
> > 
> > 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>
> > 
> 
> Did we lose track of this patchset?

Yes. I was already considering a resend.

	Torsten

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

[Theodore Ts'o]

On Thu, Jun 12, 2014 at 12:09:54PM +0200, Torsten Duwe wrote:
> > 
> > Did we lose track of this patchset?
> 
> Yes. I was already considering a resend.

I've looked it over, and I'm fairly OK with it at this point.  Do
folks mind if I just run it through the random tree?

I want to add a tracepoint for better debugging, but I can take care
of that after it's in the random tree.

						- Ted

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

[H. Peter Anvin]

Makes sense to me.

Feel free to add my

Acked-by: H. Peter Anvin <hpa@zytor.com>

On June 13, 2014 7:40:50 PM PDT, Theodore Ts'o <tytso@mit.edu> wrote:
>On Thu, Jun 12, 2014 at 12:09:54PM +0200, Torsten Duwe wrote:
>> > 
>> > Did we lose track of this patchset?
>> 
>> Yes. I was already considering a resend.
>
>I've looked it over, and I'm fairly OK with it at this point.  Do
>folks mind if I just run it through the random tree?
>
>I want to add a tracepoint for better debugging, but I can take care
>of that after it's in the random tree.
>
>						- Ted

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

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

[Theodore Ts'o]

OK, I've merged these changes into the random.git tree.

I had to make a few minor changes.

1)  Changes so it would compile on 3.15.  (random_write_wakeup_thresh
got renamed to random_write_wakeup_bits).  I'm guessing the patch was
massaged so that it would apply, but it was never compile tested.

2)  Fixed a bug in patch #2 so that it would work correctly if the rng
driver doesn't have an init function (which happens to be the case for
the tpm-rng driver, which I used for my testing).

There are also a few minor rough edges that I've noted, but not yet
fixed.  The main one is that if you've compiled the hw_random's
rng_core into the kernel, changes to
/sys/modules/rng_core/parameters/* won't actually cause the hwrngd
kerenl thread to get started.  You have to set the parameters before
you load the rng module in order for them to be activated.  And if
you've compiled the rng module into the kernel, that trick won't work.

Fixing this probably means that we need to set up a formal sysfs tree
under /sys/kernel/hw_random.

						- Ted

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

[Torsten Duwe]

On Sun, Jun 15, 2014 at 01:11:46AM -0400, Theodore Ts'o wrote:
> OK, I've merged these changes into the random.git tree.
> 
> I had to make a few minor changes.
> 
> 1)  Changes so it would compile on 3.15.  (random_write_wakeup_thresh
> got renamed to random_write_wakeup_bits).  I'm guessing the patch was
> massaged so that it would apply, but it was never compile tested.

I'm keeping and updating 2 versions, one -current (more or less)
and one for 3.12. I probably missed that when making the discussion changes
back and forth, sorry.

> 2)  Fixed a bug in patch #2 so that it would work correctly if the rng
> driver doesn't have an init function (which happens to be the case for
> the tpm-rng driver, which I used for my testing).

The whole thing stems from entropy-challenged s390. 3.12 on s390 compiles
and runs fine. Yields a solid 200 kB/s

TPM RNG is a crook ;-)

> There are also a few minor rough edges that I've noted, but not yet
> fixed.  The main one is that if you've compiled the hw_random's
> rng_core into the kernel, changes to
> /sys/modules/rng_core/parameters/* won't actually cause the hwrngd
> kerenl thread to get started.  You have to set the parameters before
> you load the rng module in order for them to be activated.  And if
> you've compiled the rng module into the kernel, that trick won't work.

With patch 03/03, it is up to the driver author to specify an entropy
quality, which can be overridden at boot time, or when loading
the module, respectively. This should be a constant hardware property.
It would be nice to change it at runtime; but frankly I hope that this
won't be neccessary.

> Fixing this probably means that we need to set up a formal sysfs tree
> under /sys/kernel/hw_random.

Maybe along with more sophisticated steering of how many bits to pick
from which source, if multiple are available.

Thanks,

	Torsten

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

[Theodore Ts'o]

On Mon, Jun 16, 2014 at 09:31:08AM +0200, Torsten Duwe wrote:
> > 2)  Fixed a bug in patch #2 so that it would work correctly if the rng
> > driver doesn't have an init function (which happens to be the case for
> > the tpm-rng driver, which I used for my testing).
> 
> The whole thing stems from entropy-challenged s390. 3.12 on s390 compiles
> and runs fine. Yields a solid 200 kB/s
> 
> TPM RNG is a crook ;-)

I think the word you mean is "crock" (as in "crock of sh*t"?)  :-)

Were you referring to the typical hardware implementation in most
TPM's, or something else?

If you're testing on s390, I'd appreciate it if you could give the "dev"
branch on random.git a quick try.  It has some improvements to help
deal with "entropy challenged" platforms by using the register values
to be mixed in with everything else.  This was based on some
suggestions and work by Jörn Engel.

>From my quick testing on x86 platforms, it doubles the overhead of
add_interrupt_randomness() on platforms that don't have a cycle
counter.  It still should be fairly low overhead, but it would be
great if some folks on other architectures did some testing to confirm
that the resulting overhead isn't problematic.  (Jörn wanted to mix in
all of the registers, but from my calculations, even if we only did
this once a clock tick, the overhead and resulting long tail latency
in interrupt processing time was not something I was willing to
inflict on everyone.)

Also note if you are playing with the random.git tree, there is a
fairly serious bug fix on the "for_linus_stable" branch that will be
pushed to Linus once it gets a bit more testing.

> With patch 03/03, it is up to the driver author to specify an entropy
> quality, which can be overridden at boot time, or when loading
> the module, respectively. This should be a constant hardware property.
> It would be nice to change it at runtime; but frankly I hope that this
> won't be neccessary.

The question of what should be the proper derating mechanism is going
to be subject to individual administrators.  I agree that we should
have good defaults, but for example, I'm sure the manufacturer of the
TPM that's in my Thinkpad would try to claim that it's the Bug
Free(tm), and try to assign it derating factor accordingly.  If the
manufacturer is supplying the device driver, it may not be a value
that other people will agree with.  Which is why I think making it
runtime configurable is a good thing.

As another example, I assume Peter or someone else from Intel will be
shortly submitting a hw_random driver for RDRAND on x86.  What should
the derating factor be for that?  I suspect David Johnson's answer
would be quite different from other people's.  And that's to be
expected, since he has much better information that most of us have
access to about the RDRAND implementation, and the
likelihood/possibiliy it could have been subverted.

> > Fixing this probably means that we need to set up a formal sysfs tree
> > under /sys/kernel/hw_random.
> 
> Maybe along with more sophisticated steering of how many bits to pick
> from which source, if multiple are available.

Yeah, the question about what to do we have multiple hw random sources
is something that I thought about.  Do we want to poll from more than
one?

Also, suppose some hw random sources require more resources ---
battery life in particular, for mobile/laptop devices?  How do we deal
with policy questions such as these?  Should we deal with it all, or
just assume that userspace will dynamically enable or disable pulling
from certain devices based on policy questions such as power
utilization issues?

Cheers,

						- Ted

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

[Torsten Duwe]

On Mon, Jun 16, 2014 at 07:22:07AM -0400, Theodore Ts'o wrote:
> On Mon, Jun 16, 2014 at 09:31:08AM +0200, Torsten Duwe wrote:
> > > 2)  Fixed a bug in patch #2 so that it would work correctly if the rng
> > > driver doesn't have an init function (which happens to be the case for
> > > the tpm-rng driver, which I used for my testing).
> > 
> > The whole thing stems from entropy-challenged s390. 3.12 on s390 compiles
> > and runs fine. Yields a solid 200 kB/s
> > 
> > TPM RNG is a crook ;-)
> 
> I think the word you mean is "crock" (as in "crock of sh*t"?)  :-)

Actually, I was thinking of a crutch. Makes you walk slowly, but better
than nothing. Seems I've bent the wrong tube.

> Were you referring to the typical hardware implementation in most
> TPM's, or something else?

Those are designed for the TPM's own, internal use IIRC. Their exposure
to the main computer is only a side effect.

> > With patch 03/03, it is up to the driver author to specify an entropy
> > quality, which can be overridden at boot time, or when loading
> > the module, respectively. This should be a constant hardware property.
> > It would be nice to change it at runtime; but frankly I hope that this
> > won't be neccessary.
> 
> The question of what should be the proper derating mechanism is going
> to be subject to individual administrators.  I agree that we should
> have good defaults, but for example, I'm sure the manufacturer of the
> TPM that's in my Thinkpad would try to claim that it's the Bug
> Free(tm), and try to assign it derating factor accordingly.  If the

Then the next question would be about the underlying specification.
A bug free implementation of dual-EC DRBG?

> manufacturer is supplying the device driver, it may not be a value
> that other people will agree with.  Which is why I think making it
> runtime configurable is a good thing.

Boot time configurable, I'd say. Again: this is a hardware property,
multiplied by the admin's level of confidence in the absence of backdoors.
It's easy with s390: from z/VM you can read all the guest's memory anyway.
If you use this machine, you already trust IBM.

> As another example, I assume Peter or someone else from Intel will be
> shortly submitting a hw_random driver for RDRAND on x86.  What should
> the derating factor be for that?  I suspect David Johnson's answer
> would be quite different from other people's.  And that's to be
> expected, since he has much better information that most of us have
> access to about the RDRAND implementation, and the
> likelihood/possibiliy it could have been subverted.

So let's keep it close to 0, and allow those to raise it who have confidence.

> > Maybe along with more sophisticated steering of how many bits to pick
> > from which source, if multiple are available.
> 
> Yeah, the question about what to do we have multiple hw random sources
> is something that I thought about.  Do we want to poll from more than
> one?

Of course! Choose your mix!

> Also, suppose some hw random sources require more resources ---
> battery life in particular, for mobile/laptop devices?  How do we deal
> with policy questions such as these?  Should we deal with it all, or
> just assume that userspace will dynamically enable or disable pulling
> from certain devices based on policy questions such as power
> utilization issues?

One thing after the other. What are the consumers of kernel entropy?
Mostly ASLR, I guess, and the web server / sshd accepting connections.
Those proceses starting probably eats more power than a HWRNG needs for
the appropriate random bits. We can address exceptions once they arise.

	Torsten

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

[Theodore Ts'o]

On Mon, Jun 16, 2014 at 04:07:19PM +0200, Torsten Duwe wrote:
> > > TPM RNG is a crook ;-)
> > 
> > I think the word you mean is "crock" (as in "crock of sh*t"?)  :-)
> 
> Actually, I was thinking of a crutch. Makes you walk slowly, but better
> than nothing. Seems I've bent the wrong tube.

Heh.  One of the things that I have considered, especially for TPM, is
that in addition to having a very small quality rating, we should also
have some kind of delay so that we sleep some small amount time before
we pull from the TPM again.

Otherwise the result of using a very small quality rating is that we
end up pounding on the TPM a huge amount until the entropy pool is
above the write_wakeup threshold.  If there's some "real" use of the
TPM, such as authenticating to a wireless network, or some such, I'd
rather not be constantly pounding on the TPM if so happens that there
is a heavy drain on /dev/random at the same time that Network Manager
needs to reauthenticate to the 802.1x network.

> > manufacturer is supplying the device driver, it may not be a value
> > that other people will agree with.  Which is why I think making it
> > runtime configurable is a good thing.
> 
> Boot time configurable, I'd say. Again: this is a hardware property,
> multiplied by the admin's level of confidence in the absence of backdoors.
> It's easy with s390: from z/VM you can read all the guest's memory anyway.
> If you use this machine, you already trust IBM.

Sure, but I guess I'm a bit allergic to gazillions of boot
command-line parameters.  I guess if you are building a modular
kernel, this matters a lot less, since you can put the configs in
/etc/modprobe.d.

						- Ted

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

[Ingo Tuchscherer]

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


Hi @all,

First, sorry for the late feedback, I was busy with other topics.

In the meanwhile I had a look at this hwrng entropy feature related to s390
and the zcrypt hardware random generator.
I have a concern about the initial call to the hwrng device while trying to
register the same.

drivers/char/hw_random/core.c
...
int hwrng_register(struct hwrng *rng) {
   ...
   bytes_read = rng_get_data(rng, bytes, sizeof(bytes), 1);
        if (bytes_read > 0)
                add_device_randomness(bytes, bytes_read);
  ...
}

At this point in time the zcrypt hw device is not registered completely in
the internal zcrypt device list (ap_device_list). The crypto card itself is
initialized and ready to receive and service requests, but the tasklet that
retrieve the card response is not able to find the device because it's not
yet in the list. Finally the response would not be received and the thread
is still waiting for completion, hence the probing/registering procedure
for all the other crypto devices is blocked. No further devices could be
scanned, initialized and registered.
Therefore I would not recommend to trigger a hw device request before the
device register process has completed.

Anyway, when the kernel thread (hwrng_fillfn) starts, it take care about
the initial call to the zcrypt rng device to be triggered. Therefore I
don't see any reason to manually call rng_get_data() during the
registration or did I missed something?

Mit freundlichen Grüßen / Kind regards

Ingo Tuchscherer

Software Development - Linux on System z
IBM Systems &Technology Group
                                                                       
                                                                       
                                                                       
                                                                       
                                                                       
 Phone:            +49-7031-16-1986                 IBM Deutschland                          (Embedded
                                                                                           image moved
                                                                                              to file:
                                                                                         pic60502.gif)
                                                                       
 E-Mail:           ingo.tuchscherer@de.ibm.com      Schoenaicher Str. 220
                                                                       
                                                    71032 Boeblingen   
                                                                       
                                                    Germany            
                                                                       
                                                                       
                                                                       
                                                                       
                                                                       
 IBM Deutschland                                                       
 Research &                                                            
 Development                                                           
 GmbH /                                                                
 Vorsitzender des                                                      
 Aufsichtsrats:                                                        
 Martina Koederitz                                                     
                                                                       
 Geschäftsführung:                                                 
 Dirk Wittkopp                                                         
 Sitz der                                                              
 Gesellschaft:                                                         
 Böblingen /                                                         
 Registergericht:                                                      
 Amtsgericht                                                           
 Stuttgart, HRB                                                        
 243294                                                                
                                                                       





From:	"Theodore Ts'o" <tytso@mit.edu>
To:	Torsten Duwe <duwe@suse.de>,
Cc:	Ingo Tuchscherer/Germany/IBM@IBMDE, Hans-Georg
            Markgraf/Germany/IBM@IBMDE, geralds@linux.vnet.ibm.com,
            mschwid2@linux.vnet.ibm.com, heicars2@linux.vnet.ibm.com, Jörn
            Engel <joern@logfs.org>
Date:	16.06.2014 16:28
Subject:	Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device



On Mon, Jun 16, 2014 at 04:14:44PM +0200, Torsten Duwe wrote:
> Also, I added the patch below to our kernel. Who wants to take that
> one-liner upstream?

This will require sucking in the "master" branch of random.git.  So
either the s390 tree can merge in the master branch, or I can carry it
in the random.git tree.  I'm happy to do that unless their are any
objections.

(Note: if you want to base something on random.git, please use the
master branch.  The dev branch is sometimes rewound...  I've since
moved master up to the dev branch, which is my guarantee not to rewind
anything up to the master branch.)

> --- linux-3.12-SLE12/drivers/s390/crypto/zcrypt_api.c.orig
2014-06-16 14:23:27.726162185 +0200
> +++ linux-3.12-SLE12/drivers/s390/crypto/zcrypt_api.c
2014-06-16 14:46:31.869103924 +0200
> @@ -1374,6 +1374,7 @@ static int zcrypt_rng_data_read(struct h
>  static struct hwrng zcrypt_rng_dev = {
>  		 .name		 		 = "zcrypt",
>  		 .data_read		 = zcrypt_rng_data_read,
> +		 .quality		 = 990,
>  };
>
>  static int zcrypt_rng_device_add(void)

I assume the z-Series RNG is presumed to be high quality, yes?

  		      		       		      		       -
Ted


[-- Attachment #2: pic60502.gif --]
[-- Type: image/gif, Size: 1851 bytes --]

Re: [Patch v5.1 03/03]: hwrng: khwrngd derating per device

[Theodore Ts'o]

On Fri, Jul 11, 2014 at 03:43:24PM +0200, Ingo Tuchscherer wrote:
> At this point in time the zcrypt hw device is not registered completely in
> the internal zcrypt device list (ap_device_list). The crypto card itself is
> initialized and ready to receive and service requests, but the tasklet that
> retrieve the card response is not able to find the device because it's not
> yet in the list. Finally the response would not be received and the thread
> is still waiting for completion, hence the probing/registering procedure
> for all the other crypto devices is blocked. No further devices could be
> scanned, initialized and registered.
> Therefore I would not recommend to trigger a hw device request before the
> device register process has completed.

Is there any chance that you could change the zcrypt driver so that it
doesn't call hwrng_register() until and the tasklet that can receive
the request has been fully registered?  It sounds like that should
just be a matter of reordering a few lines of code, no?

> Anyway, when the kernel thread (hwrng_fillfn) starts, it take care about
> the initial call to the zcrypt rng device to be triggered. Therefore I
> don't see any reason to manually call rng_get_data() during the
> registration or did I missed something?

The basic idea was described in the commit that added this:

commit d9e79726193346569af7953369a638ee2275ade5
Author: Kees Cook <keescook@chromium.org>
Date:   Mon Mar 3 15:51:48 2014 -0800

    hwrng: add randomness to system from rng sources
    
    When bringing a new RNG source online, it seems like it would make sense
    to use some of its bytes to make the system entropy pool more random,
    as done with all sorts of other devices that contain per-device or
    per-boot differences.
    
    Signed-off-by: Kees Cook <keescook@chromium.org>
    Reviewed-by: Jason Cooper <jason@lakedaemon.net>
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,

						- Ted