Re: [Qemu-devel] [PATCH v3 0/1] virtio-rng: hardware random number generator

2012-09-16 Thread H. Peter Anvin

On 06/19/2012 11:59 PM, Amit Shah wrote:

Hello,

Here's the 3rd iteration of the virtio-rng device.  This update just
rebases the patch on top of current master.

Details on the patch in the commit message.



Hi everyone...

I just stumbled on this patchset after realizing that the virtio-rng 
support still is not even in the Qemu git tree even though the kernel 
side has been there for four years(!)


It seems this support has been stuck in overengineering hell for 
years.  I have to admit to having a bit of a confusion about what is so 
hard about reading from a file descriptor, which may return partial 
reads.  I understand the fairness argument, but I would argue that if it 
is an actual problem should be solved in the kernel and therefore 
benefit all types of applications rather than at the application level 
(which Qemu, effectively, is.)


However, one key error I spotted was using /dev/urandom.  Using 
/dev/urandom is a very serious cryptographic error, as well as 
completely pointless.


The whole point of this is to provided distilled entropy to the guest, 
so that it can seed true entropy into *its* entropy pool.  As such, 
using /dev/urandom is:


a) needlessly slow.  It will churn the host kernel PRNG needlessly,
   and not provide any backpressure when the host pool is already
   drained.  Using /dev/random instead will indicate that the host
   pool is drained, and not pass a bunch of PRNG noise across an
   expensive channel when the PRNG in the *guest* could do the PRNG
   expansion just as well.

b) cryptographically incorrect.  This will tell the guest that it
   is being provided with entropy that it doesn't actually have, which
   will mean the guest severely overestimates the entropy that it has
   available to it.  To put it differently: /dev/random in the guest
   will behave like (a very expensive version of) /dev/urandom from
   a cryptographic point of view.

The right interface to the host kernel, therefore, is /dev/random.

-hpa

--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.




Re: [Qemu-devel] [PATCH v3 0/1] virtio-rng: hardware random number generator

2012-09-16 Thread Anthony Liguori
H. Peter Anvin h...@zytor.com writes:

 On 06/19/2012 11:59 PM, Amit Shah wrote:
 Hello,

 Here's the 3rd iteration of the virtio-rng device.  This update just
 rebases the patch on top of current master.

 Details on the patch in the commit message.


 Hi everyone...

 I just stumbled on this patchset after realizing that the virtio-rng 
 support still is not even in the Qemu git tree even though the kernel 
 side has been there for four years(!)

The kernel implementation was done for lguest.  No implementation was
done for QEMU.

A couple have been attempted since then.  This most recent one will go
in.  However...

 It seems this support has been stuck in overengineering hell for 
 years.  I have to admit to having a bit of a confusion about what is so 
 hard about reading from a file descriptor, which may return partial 
 reads.  I understand the fairness argument, but I would argue that if it 
 is an actual problem should be solved in the kernel and therefore 
 benefit all types of applications rather than at the application level 
 (which Qemu, effectively, is.)

 However, one key error I spotted was using /dev/urandom.  Using 
 /dev/urandom is a very serious cryptographic error, as well as 
 completely pointless.

 The whole point of this is to provided distilled entropy to the guest, 
 so that it can seed true entropy into *its* entropy pool.  As such, 
 using /dev/urandom is:

 a) needlessly slow.  It will churn the host kernel PRNG needlessly,
 and not provide any backpressure when the host pool is already
 drained.  Using /dev/random instead will indicate that the host
 pool is drained, and not pass a bunch of PRNG noise across an
 expensive channel when the PRNG in the *guest* could do the PRNG
 expansion just as well.

 b) cryptographically incorrect.  This will tell the guest that it
 is being provided with entropy that it doesn't actually have, which
 will mean the guest severely overestimates the entropy that it has
 available to it.  To put it differently: /dev/random in the guest
 will behave like (a very expensive version of) /dev/urandom from
 a cryptographic point of view.

 The right interface to the host kernel, therefore, is /dev/random.

This is *exactly* what the problem is.

If using /dev/urandom is pointless--and so far, many people have made
compelling arguments that it is--then using /dev/random is seemingly
impossible to do fairly.

The virtio-rng interface doesn't have any notion of guarantee of entropy
availability.  The guest just keeps requesting entropy with no
indication to the hypervisor of what it should and shouldn't give.

Since /dev/random is a finite pool, it's quite possible that one guest
could quickly exhaust /dev/random for all other guests.  How is this not
a clear denial of service?

This is why supporting EGD is so important IMHO.  Something else needs
to deal with handing out entropy in a responsible way.

Anyway, this is on my list for 1.3.  The series just needs some light
dusting before resubmission.

Regards,

Anthony Liguori


   -hpa

 -- 
 H. Peter Anvin, Intel Open Source Technology Center
 I work for Intel.  I don't speak on their behalf.



Re: [Qemu-devel] [PATCH v3 0/1] virtio-rng: hardware random number generator

2012-09-16 Thread H. Peter Anvin

On 09/16/2012 04:23 PM, Anthony Liguori wrote:

This is *exactly* what the problem is.

If using /dev/urandom is pointless--and so far, many people have made
compelling arguments that it is--then using /dev/random is seemingly
impossible to do fairly.


It is not merely pointless, it is a security hole.


The virtio-rng interface doesn't have any notion of guarantee of entropy
availability.  The guest just keeps requesting entropy with no
indication to the hypervisor of what it should and shouldn't give.


With the latest fixes to rngd this is no longer true.  This *was* a bug 
in rngd  4; it would always claim to be entropy-starved (and so a guest 
running rngd  4 will have this pathology, but no distro is known to run 
rngd  4 by default due to a number of other problems with these 
versions of rngd).  This has been fixed.  If that backpressure isn't 
propagated through the virtio-rng driver then that does need to be 
fixed, but from at least a cursory look I think it does.



Since /dev/random is a finite pool, it's quite possible that one guest
could quickly exhaust /dev/random for all other guests.  How is this not
a clear denial of service?


Well, by that definition the fact that the service hasn't been provided 
at all until now is a bigger denial of service...



This is why supporting EGD is so important IMHO.  Something else needs
to deal with handing out entropy in a responsible way.


No, you're missing the key point.  Fixing this in applications (and from 
the host kernel's perspective, this is an application) is broken, 
because it is not just Qemu that has that property.  If this is an 
actual problem (and it's not clear to me that it is in anything but 
theory, because although the kernel doesn't do round robin it will at 
least provide amount of stochastic fairness) then it is in the kernel 
that the fix belongs.


Throwing an extra daemon -- one which doesn't even claim to be designed 
for this purpose -- into the middle is just silly.  Either which way, it 
can easily be handled via a sane fairness daemon which doesn't need a 
complicated protocol.



Anyway, this is on my list for 1.3.  The series just needs some light
dusting before resubmission.


-hpa

--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.




Re: [Qemu-devel] [PATCH v3 0/1] virtio-rng: hardware random number generator

2012-09-16 Thread Amit Shah
On (Sun) 16 Sep 2012 [13:42:46], H. Peter Anvin wrote:
 On 06/19/2012 11:59 PM, Amit Shah wrote:
 Hello,
 
 Here's the 3rd iteration of the virtio-rng device.  This update just
 rebases the patch on top of current master.
 
 Details on the patch in the commit message.
 
 
 Hi everyone...
 
 I just stumbled on this patchset after realizing that the virtio-rng
 support still is not even in the Qemu git tree even though the
 kernel side has been there for four years(!)
 
 It seems this support has been stuck in overengineering hell for
 years.  I have to admit to having a bit of a confusion about what is
 so hard about reading from a file descriptor, which may return
 partial reads.  I understand the fairness argument, but I would
 argue that if it is an actual problem should be solved in the kernel
 and therefore benefit all types of applications rather than at the
 application level (which Qemu, effectively, is.)
 
 However, one key error I spotted was using /dev/urandom.  Using
 /dev/urandom is a very serious cryptographic error, as well as
 completely pointless.
 
 The whole point of this is to provided distilled entropy to the
 guest, so that it can seed true entropy into *its* entropy pool.  As
 such, using /dev/urandom is:
 
 a) needlessly slow.  It will churn the host kernel PRNG needlessly,
and not provide any backpressure when the host pool is already
drained.  Using /dev/random instead will indicate that the host
pool is drained, and not pass a bunch of PRNG noise across an
expensive channel when the PRNG in the *guest* could do the PRNG
expansion just as well.
 
 b) cryptographically incorrect.  This will tell the guest that it
is being provided with entropy that it doesn't actually have, which
will mean the guest severely overestimates the entropy that it has
available to it.  To put it differently: /dev/random in the guest
will behave like (a very expensive version of) /dev/urandom from
a cryptographic point of view.
 
 The right interface to the host kernel, therefore, is /dev/random.

Agreed with your points -- /dev/random on the host itself could be fed
in via a real hwrng.  Also, for the fairness arguments, several guests
doing IO also contribute to the random pool.

The ideal interface, though, in qemu should be sourcing entropy from
a file descriptor, and the admin chooses what he wants to source
entropy from - /dev/random, /dev/urandom, or even /dev/hwrng.

Amit



Re: [Qemu-devel] [PATCH v3 0/1] virtio-rng: hardware random number generator

2012-09-16 Thread H. Peter Anvin
Right, obviously.  However, under no circumstances should /dev/urandom be used!

Amit Shah amit.s...@redhat.com wrote:

On (Sun) 16 Sep 2012 [13:42:46], H. Peter Anvin wrote:
 On 06/19/2012 11:59 PM, Amit Shah wrote:
 Hello,
 
 Here's the 3rd iteration of the virtio-rng device.  This update just
 rebases the patch on top of current master.
 
 Details on the patch in the commit message.
 
 
 Hi everyone...
 
 I just stumbled on this patchset after realizing that the virtio-rng
 support still is not even in the Qemu git tree even though the
 kernel side has been there for four years(!)
 
 It seems this support has been stuck in overengineering hell for
 years.  I have to admit to having a bit of a confusion about what is
 so hard about reading from a file descriptor, which may return
 partial reads.  I understand the fairness argument, but I would
 argue that if it is an actual problem should be solved in the kernel
 and therefore benefit all types of applications rather than at the
 application level (which Qemu, effectively, is.)
 
 However, one key error I spotted was using /dev/urandom.  Using
 /dev/urandom is a very serious cryptographic error, as well as
 completely pointless.
 
 The whole point of this is to provided distilled entropy to the
 guest, so that it can seed true entropy into *its* entropy pool.  As
 such, using /dev/urandom is:
 
 a) needlessly slow.  It will churn the host kernel PRNG needlessly,
and not provide any backpressure when the host pool is already
drained.  Using /dev/random instead will indicate that the host
pool is drained, and not pass a bunch of PRNG noise across an
expensive channel when the PRNG in the *guest* could do the PRNG
expansion just as well.
 
 b) cryptographically incorrect.  This will tell the guest that it
is being provided with entropy that it doesn't actually have,
which
will mean the guest severely overestimates the entropy that it has
available to it.  To put it differently: /dev/random in the guest
will behave like (a very expensive version of) /dev/urandom from
a cryptographic point of view.
 
 The right interface to the host kernel, therefore, is /dev/random.

Agreed with your points -- /dev/random on the host itself could be fed
in via a real hwrng.  Also, for the fairness arguments, several guests
doing IO also contribute to the random pool.

The ideal interface, though, in qemu should be sourcing entropy from
a file descriptor, and the admin chooses what he wants to source
entropy from - /dev/random, /dev/urandom, or even /dev/hwrng.

   Amit

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



[Qemu-devel] [PATCH v3 0/1] virtio-rng: hardware random number generator

2012-06-20 Thread Amit Shah
Hello,

Here's the 3rd iteration of the virtio-rng device.  This update just
rebases the patch on top of current master.

Details on the patch in the commit message.

Please apply,

Amit

v3:
 * rebase to master
   * Add file to hw/Makefile.objs instead of Makefile.objs
   * Rate-limit event to at most 1 per second
   * Update to slightly different way to define new QEVENTS

v2:
 * Remove hard-wiring to /dev/urandom
 * Use chardev for input
 * Add a QMP event for notifying listeners about entropy needed and
   the bytes asked for by the guest.
 * Add s390 code


Amit Shah (1):
  virtio-rng: hardware random number generator device

 hw/Makefile.objs |1 +
 hw/pci.h |1 +
 hw/s390-virtio-bus.c |   35 +
 hw/s390-virtio-bus.h |2 +
 hw/virtio-pci.c  |   51 +
 hw/virtio-pci.h  |2 +
 hw/virtio-rng.c  |  200 ++
 hw/virtio-rng.h  |   24 ++
 hw/virtio.h  |3 +
 monitor.c|4 +-
 monitor.h|1 +
 11 files changed, 323 insertions(+), 1 deletions(-)
 create mode 100644 hw/virtio-rng.c
 create mode 100644 hw/virtio-rng.h

-- 
1.7.7.6