diff mbox

conf: Drop restrictions on rng backend path

Message ID 00a8955fd5a9503464f0e6122c413cf4f260cf3f.1461190765.git.crobinso@redhat.com
State Accepted
Commit 67f2b72723c242969c5282fcb9acf00cc01f2a54
Headers show

Commit Message

Cole Robinson April 20, 2016, 10:19 p.m. UTC
Currently we only allow /dev/random and /dev/hwrng as host input
for <rng><backend model='random'/> device. This was added after
various upstream discussions in commit 4932ef45

However this restriction has generated quite a few complaints over
the years, so a new discussion was initiated:

http://www.redhat.com/archives/libvir-list/2016-April/msg00987.html

Several people suggested removing the restriction, and nobody really
spoke up to defend it. So this patch drops the path restriction
entirely

https://bugzilla.redhat.com/show_bug.cgi?id=1074464
---
 docs/formatdomain.html.in                                     | 9 ++++-----
 docs/schemas/domaincommon.rng                                 | 3 +--
 src/conf/domain_conf.c                                        | 8 --------
 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args    | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml     | 2 +-
 tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-rng-random.xml | 2 +-
 6 files changed, 8 insertions(+), 18 deletions(-)

-- 
2.7.3

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

Comments

Cole Robinson April 21, 2016, 6 p.m. UTC | #1
On 04/21/2016 04:06 AM, Peter Krempa wrote:
> On Wed, Apr 20, 2016 at 18:19:25 -0400, Cole Robinson wrote:

>> Currently we only allow /dev/random and /dev/hwrng as host input

>> for <rng><backend model='random'/> device. This was added after

>> various upstream discussions in commit 4932ef45

>>

>> However this restriction has generated quite a few complaints over

>> the years, so a new discussion was initiated:

>>

>> http://www.redhat.com/archives/libvir-list/2016-April/msg00987.html

>>

>> Several people suggested removing the restriction, and nobody really

>> spoke up to defend it. So this patch drops the path restriction

>> entirely

>>

>> https://bugzilla.redhat.com/show_bug.cgi?id=1074464

>> ---

>>  docs/formatdomain.html.in                                     | 9 ++++-----

>>  docs/schemas/domaincommon.rng                                 | 3 +--

>>  src/conf/domain_conf.c                                        | 8 --------

>>  tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args    | 2 +-

>>  tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml     | 2 +-

>>  tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-rng-random.xml | 2 +-

>>  6 files changed, 8 insertions(+), 18 deletions(-)

>>

>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in

>> index 895114b..132b261 100644

>> --- a/docs/formatdomain.html.in

>> +++ b/docs/formatdomain.html.in

> 

> ...

> 

>> @@ -6240,9 +6239,9 @@ qemu-kvm -net nic,model=? /dev/null

>>        <dd>

>>          <p>

>>            This backend type expects a non-blocking character device as input.

>> -          The only accepted paths are /dev/random and /dev/hwrng. The file

>> -          name is specified as contents of the <code>backend</code> element.

>> -          When no file name is specified the hypervisor default is used.

>> +          The file name is specified as contents of the

>> +          <code>backend</code> element. When no file name is specified the

>> +          this defaults to /dev/random.

> 

> I'd rather keep the "hypervisor default" wording. /dev/urandom is in no

> way platform or hypervisor agnostic. You may add though that for qemu

> driver the default indeed is /dev/random.

> 


Thanks, fixed locally. I'll wait till see if any other comments come in before
deciding whether to post a V2 or not

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Cole Robinson April 26, 2016, 3:48 p.m. UTC | #2
On 04/26/2016 11:07 AM, Daniel P. Berrange wrote:
> On Wed, Apr 20, 2016 at 06:19:25PM -0400, Cole Robinson wrote:

>> Currently we only allow /dev/random and /dev/hwrng as host input

>> for <rng><backend model='random'/> device. This was added after

>> various upstream discussions in commit 4932ef45

>>

>> However this restriction has generated quite a few complaints over

>> the years, so a new discussion was initiated:

>>

>> http://www.redhat.com/archives/libvir-list/2016-April/msg00987.html

>>

>> Several people suggested removing the restriction, and nobody really

>> spoke up to defend it. So this patch drops the path restriction

>> entirely

> 

> ACK, despite explicit request for details, no one has been able to

> give a clear description of a security problem in using urandom.

> It has all just been hand-wavey assertions with nothing to back

> it up, against other people's analysis showing urandom to be safe.

> 


Thanks, I've pushed the patch now

- Cole

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

Patch

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 895114b..132b261 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6231,8 +6231,7 @@  qemu-kvm -net nic,model=? /dev/null
           <code>model</code> attribute. Supported source models are:
         </p>
         <ul>
-          <li>'random' &mdash; /dev/random (default) or /dev/hwrng
-            device as source (for now, no other sources are permitted)</li>
+          <li>'random' &mdash; a character device backend</li>
           <li>'egd' &mdash; a EGD protocol backend</li>
         </ul>
       </dd>
@@ -6240,9 +6239,9 @@  qemu-kvm -net nic,model=? /dev/null
       <dd>
         <p>
           This backend type expects a non-blocking character device as input.
-          The only accepted paths are /dev/random and /dev/hwrng. The file
-          name is specified as contents of the <code>backend</code> element.
-          When no file name is specified the hypervisor default is used.
+          The file name is specified as contents of the
+          <code>backend</code> element. When no file name is specified the
+          this defaults to /dev/random.
         </p>
       </dd>
       <dt><code>backend model='egd'</code></dt>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 3605afe..c91a3a9 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4691,8 +4691,7 @@ 
             <value>random</value>
           </attribute>
           <choice>
-            <value>/dev/random</value>
-            <value>/dev/hwrng</value>
+            <ref name='absFilePath'/>
             <empty/>
           </choice>
         </group>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 28248c8..f9cd69c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11512,14 +11512,6 @@  virDomainRNGDefParseXML(xmlNodePtr node,
     switch ((virDomainRNGBackend) def->backend) {
     case VIR_DOMAIN_RNG_BACKEND_RANDOM:
         def->source.file = virXPathString("string(./backend)", ctxt);
-        if (def->source.file &&
-            STRNEQ(def->source.file, "/dev/random") &&
-            STRNEQ(def->source.file, "/dev/hwrng")) {
-            virReportError(VIR_ERR_XML_ERROR,
-                           _("file '%s' is not a supported random source"),
-                           def->source.file);
-            goto error;
-        }
         break;
 
     case VIR_DOMAIN_RNG_BACKEND_EGD:
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args
index ba97c1e..5a4d47b 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args
@@ -18,6 +18,6 @@  QEMU_AUDIO_DRV=none \
 -boot c \
 -usb \
 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \
--object rng-random,id=objrng0,filename=/dev/hwrng \
+-object rng-random,id=objrng0,filename=/dev/urandom \
 -device virtio-rng-pci,rng=objrng0,id=rng0,max-bytes=123,period=1234,bus=pci.0,\
 addr=0x4
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
index 71bd21a..a6e91ff 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
@@ -21,7 +21,7 @@ 
     <memballoon model='virtio'/>
     <rng model='virtio'>
       <rate bytes='123' period='1234'/>
-      <backend model='random'>/dev/hwrng</backend>
+      <backend model='random'>/dev/urandom</backend>
     </rng>
   </devices>
 </domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-rng-random.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-rng-random.xml
index e8a0478..0bdefde 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-rng-random.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-rng-random.xml
@@ -25,7 +25,7 @@ 
     </memballoon>
     <rng model='virtio'>
       <rate bytes='123' period='1234'/>
-      <backend model='random'>/dev/hwrng</backend>
+      <backend model='random'>/dev/urandom</backend>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
     </rng>
   </devices>