[Qemu-devel] [PATCH] rng: restrict passthrough names to known-good files

[Qemu-devel] [PATCH] rng: restrict passthrough names to known-good files

[Eric Blake]

There is some controversy[1] on the qemu list on whether qemu should
have ever allowed arbitrary file name passthrough, or whether it
should be restricted to JUST /dev/random and /dev/hwrng.  It is
always easier to add support for additional filenames than it is
to remove support for something once released, so this patch
restricts libvirt 1.0.3 (where the virtio-random backend was first
supported) to just the two uncontroversial names, letting us defer
to a later date any decision on whether supporting arbitrary files
makes sense. Additionally, since qemu 1.4 does NOT support
/dev/fdset/nnn fd passthrough for the backend, limiting to just
two known names means that we don't get tempted to try fd
passthrough where it won't work.

[1]https://lists.gnu.org/archive/html/qemu-devel/2013-03/threads.html#00023

* src/conf/domain_conf.c (virDomainRNGDefParseXML): Only allow
/dev/random and /dev/hwrng.
* docs/schemas/domaincommon.rng: Flag invalid files.
* docs/formatdomain.html.in (elementsRng): Document this.
* tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args:
Update test to match.
* tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml:
Likewise.
---

This needs to be acked before libvirt 1.0.3; otherwise we are
stuck supporting arbitrary name passthrough.

 docs/formatdomain.html.in                                  | 3 ++-
 docs/schemas/domaincommon.rng                              | 5 ++++-
 src/conf/domain_conf.c                                     | 7 +++++++
 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml  | 2 +-
 5 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 1835b39..4cafc92 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4310,7 +4310,8 @@ qemu-kvm -net nic,model=? /dev/null
           <code>model</code> attribute. Supported source models are:
         </p>
         <ul>
-          <li>'random' &mdash; /dev/random (default) or similar device as source</li>
+          <li>'random' &mdash; /dev/random (default) or /dev/hwrng
+            device as source (for now, no other sources are permitted)</li>
           <li>'egd' &mdash; a EGD protocol backend</li>
         </ul>
       </dd>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index e7231cc..4b60885 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3511,7 +3511,10 @@
           <attribute name="model">
             <value>random</value>
           </attribute>
-          <ref name="filePath"/>
+          <choice>
+            <value>/dev/random</value>
+            <value>/dev/hwrng</value>
+          </choice>
         </group>
         <group>
           <attribute name="model">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 995cf0c..9c96cf1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7423,6 +7423,13 @@ virDomainRNGDefParseXML(const xmlNodePtr node,
     switch ((enum virDomainRNGBackend) def->backend) {
     case VIR_DOMAIN_RNG_BACKEND_RANDOM:
         def->source.file = virXPathString("string(./backend)", ctxt);
+        if (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 ad27132..7ab9dbc 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args
@@ -2,5 +2,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu \
 -S -M pc -m 214 -smp 1 -nographic -nodefaults \
 -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \
 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \
--object 'rng-random,id=rng0,filename=/test/ph<ile' \
+-object rng-random,id=rng0,filename=/dev/hwrng \
 -device virtio-rng-pci,rng=rng0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
index 0658f4b..1e2c4be 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
@@ -17,7 +17,7 @@
     <controller type='usb' index='0'/>
     <memballoon model='virtio'/>
     <rng model='virtio'>
-      <backend model='random'>/test/ph&lt;ile</backend>
+      <backend model='random'>/dev/hwrng</backend>
     </rng>
   </devices>
 </domain>
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH] rng: restrict passthrough names to known-good files

[Anthony Liguori]

Eric Blake <eblake@redhat.com> writes:

> There is some controversy[1] on the qemu list on whether qemu should
> have ever allowed arbitrary file name passthrough, or whether it
> should be restricted to JUST /dev/random and /dev/hwrng.  It is
> always easier to add support for additional filenames than it is
> to remove support for something once released, so this patch
> restricts libvirt 1.0.3 (where the virtio-random backend was first
> supported) to just the two uncontroversial names, letting us defer
> to a later date any decision on whether supporting arbitrary files
> makes sense. Additionally, since qemu 1.4 does NOT support
> /dev/fdset/nnn fd passthrough for the backend, limiting to just
> two known names means that we don't get tempted to try fd
> passthrough where it won't work.

Acked-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

>
> [1]https://lists.gnu.org/archive/html/qemu-devel/2013-03/threads.html#00023
>
> * src/conf/domain_conf.c (virDomainRNGDefParseXML): Only allow
> /dev/random and /dev/hwrng.
> * docs/schemas/domaincommon.rng: Flag invalid files.
> * docs/formatdomain.html.in (elementsRng): Document this.
> * tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args:
> Update test to match.
> * tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml:
> Likewise.
> ---
>
> This needs to be acked before libvirt 1.0.3; otherwise we are
> stuck supporting arbitrary name passthrough.
>
>  docs/formatdomain.html.in                                  | 3 ++-
>  docs/schemas/domaincommon.rng                              | 5 ++++-
>  src/conf/domain_conf.c                                     | 7 +++++++
>  tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args | 2 +-
>  tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml  | 2 +-
>  5 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 1835b39..4cafc92 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -4310,7 +4310,8 @@ qemu-kvm -net nic,model=? /dev/null
>            <code>model</code> attribute. Supported source models are:
>          </p>
>          <ul>
> -          <li>'random' &mdash; /dev/random (default) or similar device as source</li>
> +          <li>'random' &mdash; /dev/random (default) or /dev/hwrng
> +            device as source (for now, no other sources are permitted)</li>
>            <li>'egd' &mdash; a EGD protocol backend</li>
>          </ul>
>        </dd>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index e7231cc..4b60885 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3511,7 +3511,10 @@
>            <attribute name="model">
>              <value>random</value>
>            </attribute>
> -          <ref name="filePath"/>
> +          <choice>
> +            <value>/dev/random</value>
> +            <value>/dev/hwrng</value>
> +          </choice>
>          </group>
>          <group>
>            <attribute name="model">
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 995cf0c..9c96cf1 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7423,6 +7423,13 @@ virDomainRNGDefParseXML(const xmlNodePtr node,
>      switch ((enum virDomainRNGBackend) def->backend) {
>      case VIR_DOMAIN_RNG_BACKEND_RANDOM:
>          def->source.file = virXPathString("string(./backend)", ctxt);
> +        if (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 ad27132..7ab9dbc 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args
> @@ -2,5 +2,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu \
>  -S -M pc -m 214 -smp 1 -nographic -nodefaults \
>  -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \
>  -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \
> --object 'rng-random,id=rng0,filename=/test/ph<ile' \
> +-object rng-random,id=rng0,filename=/dev/hwrng \
>  -device virtio-rng-pci,rng=rng0,bus=pci.0,addr=0x4
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
> index 0658f4b..1e2c4be 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
> @@ -17,7 +17,7 @@
>      <controller type='usb' index='0'/>
>      <memballoon model='virtio'/>
>      <rng model='virtio'>
> -      <backend model='random'>/test/ph&lt;ile</backend>
> +      <backend model='random'>/dev/hwrng</backend>
>      </rng>
>    </devices>
>  </domain>
> -- 
> 1.8.1.4

Re: [Qemu-devel] [PATCH] rng: restrict passthrough names to known-good files

[Eric Blake]

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

On 03/04/2013 04:31 PM, Anthony Liguori wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> There is some controversy[1] on the qemu list on whether qemu should
>> have ever allowed arbitrary file name passthrough, or whether it
>> should be restricted to JUST /dev/random and /dev/hwrng.  It is
>> always easier to add support for additional filenames than it is
>> to remove support for something once released, so this patch
>> restricts libvirt 1.0.3 (where the virtio-random backend was first
>> supported) to just the two uncontroversial names, letting us defer
>> to a later date any decision on whether supporting arbitrary files
>> makes sense. Additionally, since qemu 1.4 does NOT support
>> /dev/fdset/nnn fd passthrough for the backend, limiting to just
>> two known names means that we don't get tempted to try fd
>> passthrough where it won't work.
> 
> Acked-by: Anthony Liguori <aliguori@us.ibm.com>
> 


Thanks; I've pushed this into libvirt.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]