Re: [libvirt] [PATCH] Add hw random number generator (/dev/hwrng) to default qemu cgroup ACL

2013-11-18 Thread Pradipta Kumar Banerjee
On 11/18/2013 08:45 PM, Eric Blake wrote:
> On 11/18/2013 06:28 AM, Pradipta Kr. Banerjee wrote:
>> Creating a qemu VM with /dev/hwrng as backed RNG device throws the
>> following error - "Could not open '/dev/hwrng': Operation not permitted"
>> This patch fixes the issue
>>
>> Signed-off-by: Pradipta Kr. Banerjee 
>> ---
>>  src/qemu/qemu_cgroup.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> This is probably not the right fix.  This says that we are making the
> random device to all possible guests, and that a compromised guest can
> then open() the device and starve it of entropy to the detriment of
> other guests.  I think the _real_ fix is to quit exposing /dev/random by
> default (/dev/urandom may be okay), and to instead teach the code for
> RNG devices to add an ACL change for either random device (/dev/random
> or /dev/hwrng) only for the guests that actually use RNG backed by a
> hardware device.  That way, guests not using an RNG device cannot starve
> entropy from guests that depend on it.
Right..Thanks for the suggestion Eric.

> 
>>
>> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
>> index ace7e35..d9ebb30 100644
>> --- a/src/qemu/qemu_cgroup.c
>> +++ b/src/qemu/qemu_cgroup.c
>> @@ -39,7 +39,7 @@
>>  
>>  static const char *const defaultDeviceACL[] = {
>>  "/dev/null", "/dev/full", "/dev/zero",
>> -"/dev/random", "/dev/urandom",
>> +"/dev/random", "/dev/urandom", "/dev/hwrng",
>>  "/dev/ptmx", "/dev/kvm", "/dev/kqemu",
>>  "/dev/rtc", "/dev/hpet", "/dev/vfio/vfio",
>>  NULL,
>>
> 


-- 
Regards,
Pradipta

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] Add hw random number generator (/dev/hwrng) to default qemu cgroup ACL

2013-11-18 Thread Pradipta Kumar Banerjee
On 11/18/2013 09:35 PM, Eric Blake wrote:
> On 11/18/2013 08:20 AM, Daniel P. Berrange wrote:
> 
>>>  static const char *const defaultDeviceACL[] = {
>>>  "/dev/null", "/dev/full", "/dev/zero",
>>> -"/dev/random", "/dev/urandom",
>>> +"/dev/random", "/dev/urandom", "/dev/hwrng",
>>>  "/dev/ptmx", "/dev/kvm", "/dev/kqemu",
>>>  "/dev/rtc", "/dev/hpet", "/dev/vfio/vfio",
>>>  NULL,
>>
>> NACK, for any device listed in the XML, we should add it in the per-VM
>> cgroups setup code.
>>
>> The existing /dev/random & /dev/urandom devices are there because they
>> are used for basic crypto libraries unrelated to the XML config.
> 
> /dev/urandom, probably.  /dev/random - really?  Isn't that a DoS
> potential for one guest to starve entropy from other guests, in spite of
> sVirt?
I think you are right ..

> 
>> The
>> /dev/hwrng device does not fall into this scenario.
> 
> Agreed.
> 


-- 
Regards,
Pradipta

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] Add hw random number generator (/dev/hwrng) to default qemu cgroup ACL

2013-11-18 Thread Daniel P. Berrange
On Mon, Nov 18, 2013 at 09:05:21AM -0700, Eric Blake wrote:
> On 11/18/2013 08:20 AM, Daniel P. Berrange wrote:
> 
> >>  static const char *const defaultDeviceACL[] = {
> >>  "/dev/null", "/dev/full", "/dev/zero",
> >> -"/dev/random", "/dev/urandom",
> >> +"/dev/random", "/dev/urandom", "/dev/hwrng",
> >>  "/dev/ptmx", "/dev/kvm", "/dev/kqemu",
> >>  "/dev/rtc", "/dev/hpet", "/dev/vfio/vfio",
> >>  NULL,
> > 
> > NACK, for any device listed in the XML, we should add it in the per-VM
> > cgroups setup code.
> > 
> > The existing /dev/random & /dev/urandom devices are there because they
> > are used for basic crypto libraries unrelated to the XML config.
> 
> /dev/urandom, probably.  /dev/random - really?  Isn't that a DoS
> potential for one guest to starve entropy from other guests, in spite of
> sVirt?

It is a tradeoff between providing the crypto libraries a weaker
entropy source vs DOS potential via entropy starvation. Given that
/dev/random is world writable on Linux in general, I think it is
acceptable to allow QEMU access by default. Paranoid admins can
always set the cgroups device ACL in /etc/libvirt/.qemu.conf if
they want to restrict it.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] Add hw random number generator (/dev/hwrng) to default qemu cgroup ACL

2013-11-18 Thread Eric Blake
On 11/18/2013 08:20 AM, Daniel P. Berrange wrote:

>>  static const char *const defaultDeviceACL[] = {
>>  "/dev/null", "/dev/full", "/dev/zero",
>> -"/dev/random", "/dev/urandom",
>> +"/dev/random", "/dev/urandom", "/dev/hwrng",
>>  "/dev/ptmx", "/dev/kvm", "/dev/kqemu",
>>  "/dev/rtc", "/dev/hpet", "/dev/vfio/vfio",
>>  NULL,
> 
> NACK, for any device listed in the XML, we should add it in the per-VM
> cgroups setup code.
> 
> The existing /dev/random & /dev/urandom devices are there because they
> are used for basic crypto libraries unrelated to the XML config.

/dev/urandom, probably.  /dev/random - really?  Isn't that a DoS
potential for one guest to starve entropy from other guests, in spite of
sVirt?

> The
> /dev/hwrng device does not fall into this scenario.

Agreed.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] Add hw random number generator (/dev/hwrng) to default qemu cgroup ACL

2013-11-18 Thread Daniel P. Berrange
On Mon, Nov 18, 2013 at 06:58:06PM +0530, Pradipta Kr. Banerjee wrote:
> Creating a qemu VM with /dev/hwrng as backed RNG device throws the
> following error - "Could not open '/dev/hwrng': Operation not permitted"
> This patch fixes the issue
> 
> Signed-off-by: Pradipta Kr. Banerjee 
> ---
>  src/qemu/qemu_cgroup.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index ace7e35..d9ebb30 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -39,7 +39,7 @@
>  
>  static const char *const defaultDeviceACL[] = {
>  "/dev/null", "/dev/full", "/dev/zero",
> -"/dev/random", "/dev/urandom",
> +"/dev/random", "/dev/urandom", "/dev/hwrng",
>  "/dev/ptmx", "/dev/kvm", "/dev/kqemu",
>  "/dev/rtc", "/dev/hpet", "/dev/vfio/vfio",
>  NULL,

NACK, for any device listed in the XML, we should add it in the per-VM
cgroups setup code.

The existing /dev/random & /dev/urandom devices are there because they
are used for basic crypto libraries unrelated to the XML config. The
/dev/hwrng device does not fall into this scenario.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] Add hw random number generator (/dev/hwrng) to default qemu cgroup ACL

2013-11-18 Thread Eric Blake
On 11/18/2013 06:28 AM, Pradipta Kr. Banerjee wrote:
> Creating a qemu VM with /dev/hwrng as backed RNG device throws the
> following error - "Could not open '/dev/hwrng': Operation not permitted"
> This patch fixes the issue
> 
> Signed-off-by: Pradipta Kr. Banerjee 
> ---
>  src/qemu/qemu_cgroup.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

This is probably not the right fix.  This says that we are making the
random device to all possible guests, and that a compromised guest can
then open() the device and starve it of entropy to the detriment of
other guests.  I think the _real_ fix is to quit exposing /dev/random by
default (/dev/urandom may be okay), and to instead teach the code for
RNG devices to add an ACL change for either random device (/dev/random
or /dev/hwrng) only for the guests that actually use RNG backed by a
hardware device.  That way, guests not using an RNG device cannot starve
entropy from guests that depend on it.

> 
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index ace7e35..d9ebb30 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -39,7 +39,7 @@
>  
>  static const char *const defaultDeviceACL[] = {
>  "/dev/null", "/dev/full", "/dev/zero",
> -"/dev/random", "/dev/urandom",
> +"/dev/random", "/dev/urandom", "/dev/hwrng",
>  "/dev/ptmx", "/dev/kvm", "/dev/kqemu",
>  "/dev/rtc", "/dev/hpet", "/dev/vfio/vfio",
>  NULL,
> 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Add hw random number generator (/dev/hwrng) to default qemu cgroup ACL

2013-11-18 Thread Pradipta Kr. Banerjee
Creating a qemu VM with /dev/hwrng as backed RNG device throws the
following error - "Could not open '/dev/hwrng': Operation not permitted"
This patch fixes the issue

Signed-off-by: Pradipta Kr. Banerjee 
---
 src/qemu/qemu_cgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index ace7e35..d9ebb30 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -39,7 +39,7 @@
 
 static const char *const defaultDeviceACL[] = {
 "/dev/null", "/dev/full", "/dev/zero",
-"/dev/random", "/dev/urandom",
+"/dev/random", "/dev/urandom", "/dev/hwrng",
 "/dev/ptmx", "/dev/kvm", "/dev/kqemu",
 "/dev/rtc", "/dev/hpet", "/dev/vfio/vfio",
 NULL,
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list