All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
@ 2019-05-29 14:31 Laurent Vivier
  2019-05-29 14:31 ` [Qemu-devel] [PATCH v7 1/4] VirtIO-RNG: Update default entropy source to `/dev/urandom` Laurent Vivier
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Laurent Vivier @ 2019-05-29 14:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Daniel P . Berrangé,
	Kashyap Chamarthy, Amit Shah, Richard Henderson,
	Richard W . M . Jones, Markus Armbruster, Michael S. Tsirkin

Add a new RNG backend using QEMU builtin getrandom function.

v7: rebase on master
    Make rng-builtin asynchronous with QEMUBH (removed existing R-b)

v6: remove "sysemu/rng-random.h" from virtio-rng.c
    rebase on qemu_getrandom v8

v5: PATCH 1 s/linux/Linux/
    remove superfluous includes from rng-builtin.c
    don't update rng-random documentation
    add a patch from Markus to keep the default backend out of VirtIORNGConf
    move TYPE_RNG_BUILTIN to sysemu/rng.h and remove sysemu/rng-builtin.h

v4: update PATCH 1 commit message

v3: Include Kashyap's patch in the series
    Add a patch to change virtio-rng default backend to rng-builtin

v2: Update qemu-options.hx
    describe the new backend and specify virtio-rng uses the
    rng-random by default

Kashyap Chamarthy (1):
  VirtIO-RNG: Update default entropy source to `/dev/urandom`

Laurent Vivier (2):
  rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
  virtio-rng: change default backend to rng-builtin

Markus Armbruster (1):
  virtio-rng: Keep the default backend out of VirtIORNGConf

 backends/Makefile.objs         |  2 +-
 backends/rng-builtin.c         | 77 ++++++++++++++++++++++++++++++++++
 backends/rng-random.c          |  2 +-
 hw/virtio/virtio-rng.c         | 19 ++++-----
 include/hw/virtio/virtio-rng.h |  2 -
 include/sysemu/rng.h           |  2 +
 qemu-options.hx                |  9 +++-
 7 files changed, 97 insertions(+), 16 deletions(-)
 create mode 100644 backends/rng-builtin.c

-- 
2.20.1



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v7 1/4] VirtIO-RNG: Update default entropy source to `/dev/urandom`
  2019-05-29 14:31 [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom() Laurent Vivier
@ 2019-05-29 14:31 ` Laurent Vivier
  2019-06-26 20:29   ` Laurent Vivier
                     ` (2 more replies)
  2019-05-29 14:31 ` [Qemu-devel] [PATCH v7 2/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom() Laurent Vivier
                   ` (5 subsequent siblings)
  6 siblings, 3 replies; 20+ messages in thread
From: Laurent Vivier @ 2019-05-29 14:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Daniel P . Berrangé,
	Kashyap Chamarthy, Amit Shah, Richard Henderson,
	Richard W . M . Jones, Markus Armbruster, Michael S. Tsirkin,
	Stefan Hajnoczi

From: Kashyap Chamarthy <kchamart@redhat.com>

When QEMU exposes a VirtIO-RNG device to the guest, that device needs a
source of entropy, and that source needs to be "non-blocking", like
`/dev/urandom`.  However, currently QEMU defaults to the problematic
`/dev/random`, which on Linux is "blocking" (as in, it waits until
sufficient entropy is available).

Why prefer `/dev/urandom` over `/dev/random`?
---------------------------------------------

The man pages of urandom(4) and random(4) state:

    "The /dev/random device is a legacy interface which dates back to a
    time where the cryptographic primitives used in the implementation
    of /dev/urandom were not widely trusted.  It will return random
    bytes only within the estimated number of bits of fresh noise in the
    entropy pool, blocking if necessary.  /dev/random is suitable for
    applications that need high quality randomness, and can afford
    indeterminate delays."

Further, the "Usage" section of the said man pages state:

    "The /dev/random interface is considered a legacy interface, and
    /dev/urandom is preferred and sufficient in all use cases, with the
    exception of applications which require randomness during early boot
    time; for these applications, getrandom(2) must be used instead,
    because it will block until the entropy pool is initialized.

    "If a seed file is saved across reboots as recommended below (all
    major Linux distributions have done this since 2000 at least), the
    output is cryptographically secure against attackers without local
    root access as soon as it is reloaded in the boot sequence, and
    perfectly adequate for network encryption session keys.  Since reads
    from /dev/random may block, users will usually want to open it in
    nonblocking mode (or perform a read with timeout), and provide some
    sort of user notification if the desired entropy is not immediately
    available."

And refer to random(7) for a comparison of `/dev/random` and
`/dev/urandom`.

What about other OSes?
----------------------

`/dev/urandom` exists and works on OS-X, FreeBSD, DragonFlyBSD, NetBSD
and OpenBSD, which cover all the non-Linux platforms we explicitly
support, aside from Windows.

On Windows `/dev/random` doesn't work either so we don't regress.
This is actually another argument in favour of using the newly
proposed 'rng-builtin' backend by default, as that will work on
Windows.

    - - -

Given the above, change the entropy source for VirtIO-RNG device to
`/dev/urandom`.

Related discussion in these[1][2] past threads.

[1] https://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg08335.html
    -- "RNG: Any reason QEMU doesn't default to `/dev/urandom`?"
[2] https://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg02724.html
    -- "[RFC] Virtio RNG: Consider changing the default entropy source to
       /dev/urandom"

Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 backends/rng-random.c | 2 +-
 qemu-options.hx       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/backends/rng-random.c b/backends/rng-random.c
index e2a49b0571d7..eff36ef14084 100644
--- a/backends/rng-random.c
+++ b/backends/rng-random.c
@@ -112,7 +112,7 @@ static void rng_random_init(Object *obj)
                             rng_random_set_filename,
                             NULL);
 
-    s->filename = g_strdup("/dev/random");
+    s->filename = g_strdup("/dev/urandom");
     s->fd = -1;
 }
 
diff --git a/qemu-options.hx b/qemu-options.hx
index 39dc17042967..f6e9bd1d9c42 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4328,7 +4328,7 @@ Creates a random number generator backend which obtains entropy from
 a device on the host. The @option{id} parameter is a unique ID that
 will be used to reference this entropy backend from the @option{virtio-rng}
 device. The @option{filename} parameter specifies which file to obtain
-entropy from and if omitted defaults to @option{/dev/random}.
+entropy from and if omitted defaults to @option{/dev/urandom}.
 
 @item -object rng-egd,id=@var{id},chardev=@var{chardevid}
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v7 2/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
  2019-05-29 14:31 [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom() Laurent Vivier
  2019-05-29 14:31 ` [Qemu-devel] [PATCH v7 1/4] VirtIO-RNG: Update default entropy source to `/dev/urandom` Laurent Vivier
@ 2019-05-29 14:31 ` Laurent Vivier
  2019-06-06  4:46   ` Markus Armbruster
  2019-05-29 14:31 ` [Qemu-devel] [PATCH v7 3/4] virtio-rng: Keep the default backend out of VirtIORNGConf Laurent Vivier
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Laurent Vivier @ 2019-05-29 14:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Daniel P . Berrangé,
	Kashyap Chamarthy, Amit Shah, Richard Henderson,
	Richard W . M . Jones, Markus Armbruster, Michael S. Tsirkin

Add a new RNG backend using QEMU builtin getrandom function.

It can be created and used with something like:

    ... -object rng-builtin,id=rng0 -device virtio-rng,rng=rng0 ...

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 backends/Makefile.objs |  2 +-
 backends/rng-builtin.c | 78 ++++++++++++++++++++++++++++++++++++++++++
 qemu-options.hx        |  7 ++++
 3 files changed, 86 insertions(+), 1 deletion(-)
 create mode 100644 backends/rng-builtin.c

diff --git a/backends/Makefile.objs b/backends/Makefile.objs
index 981e8e122f2c..f0691116e86e 100644
--- a/backends/Makefile.objs
+++ b/backends/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += rng.o rng-egd.o
+common-obj-y += rng.o rng-egd.o rng-builtin.o
 common-obj-$(CONFIG_POSIX) += rng-random.o
 
 common-obj-$(CONFIG_TPM) += tpm.o
diff --git a/backends/rng-builtin.c b/backends/rng-builtin.c
new file mode 100644
index 000000000000..3381d47174df
--- /dev/null
+++ b/backends/rng-builtin.c
@@ -0,0 +1,78 @@
+/*
+ * QEMU Builtin Random Number Generator Backend
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/rng.h"
+#include "qemu/main-loop.h"
+#include "qemu/guest-random.h"
+
+#define TYPE_RNG_BUILTIN "rng-builtin"
+#define RNG_BUILTIN(obj) OBJECT_CHECK(RngBuiltin, (obj), TYPE_RNG_BUILTIN)
+
+typedef struct RngBuiltin {
+    RngBackend parent;
+    QEMUBH *bh;
+} RngBuiltin;
+
+static void rng_builtin_receive_entropy_bh(void *opaque)
+{
+    RngBuiltin *s = opaque;
+
+    while (!QSIMPLEQ_EMPTY(&s->parent.requests)) {
+        RngRequest *req = QSIMPLEQ_FIRST(&s->parent.requests);
+
+        qemu_guest_getrandom_nofail(req->data, req->size);
+
+        req->receive_entropy(req->opaque, req->data, req->size);
+
+        rng_backend_finalize_request(&s->parent, req);
+    }
+}
+
+static void rng_builtin_request_entropy(RngBackend *b, RngRequest *req)
+{
+    RngBuiltin *s = RNG_BUILTIN(b);
+
+    qemu_bh_schedule(s->bh);
+}
+
+static void rng_builtin_init(Object *obj)
+{
+    RngBuiltin *s = RNG_BUILTIN(obj);
+
+    s->bh = qemu_bh_new(rng_builtin_receive_entropy_bh, s);
+}
+
+static void rng_builtin_finalize(Object *obj)
+{
+    RngBuiltin *s = RNG_BUILTIN(obj);
+
+    qemu_bh_delete(s->bh);
+}
+
+static void rng_builtin_class_init(ObjectClass *klass, void *data)
+{
+    RngBackendClass *rbc = RNG_BACKEND_CLASS(klass);
+
+    rbc->request_entropy = rng_builtin_request_entropy;
+}
+
+static const TypeInfo rng_builtin_info = {
+    .name = TYPE_RNG_BUILTIN,
+    .parent = TYPE_RNG_BACKEND,
+    .instance_size = sizeof(RngBuiltin),
+    .instance_init = rng_builtin_init,
+    .instance_finalize = rng_builtin_finalize,
+    .class_init = rng_builtin_class_init,
+};
+
+static void register_types(void)
+{
+    type_register_static(&rng_builtin_info);
+}
+
+type_init(register_types);
diff --git a/qemu-options.hx b/qemu-options.hx
index f6e9bd1d9c42..4e6a6828d7ce 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4322,6 +4322,13 @@ other options.
 
 The @option{share} boolean option is @var{on} by default with memfd.
 
+@item -object rng-builtin,id=@var{id}
+
+Creates a random number generator backend which obtains entropy from
+QEMU builtin functions. The @option{id} parameter is a unique ID that
+will be used to reference this entropy backend from the @option{virtio-rng}
+device.
+
 @item -object rng-random,id=@var{id},filename=@var{/dev/random}
 
 Creates a random number generator backend which obtains entropy from
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v7 3/4] virtio-rng: Keep the default backend out of VirtIORNGConf
  2019-05-29 14:31 [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom() Laurent Vivier
  2019-05-29 14:31 ` [Qemu-devel] [PATCH v7 1/4] VirtIO-RNG: Update default entropy source to `/dev/urandom` Laurent Vivier
  2019-05-29 14:31 ` [Qemu-devel] [PATCH v7 2/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom() Laurent Vivier
@ 2019-05-29 14:31 ` Laurent Vivier
  2019-05-29 14:31 ` [Qemu-devel] [PATCH v7 4/4] virtio-rng: change default backend to rng-builtin Laurent Vivier
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Laurent Vivier @ 2019-05-29 14:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Daniel P . Berrangé,
	Kashyap Chamarthy, Amit Shah, Richard Henderson,
	Richard W . M . Jones, Markus Armbruster, Michael S. Tsirkin

From: Markus Armbruster <armbru@redhat.com>

The default backend is only used within virtio_rng_device_realize().
Replace VirtIORNGConf member default_backend by a local variable.
Adjust its type to reduce conversions.

While there, pass &error_abort instead of NULL when failure would be a
programming error.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/virtio/virtio-rng.c         | 20 +++++++++-----------
 include/hw/virtio/virtio-rng.h |  2 --
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index 30493a258622..73ffb476e030 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -16,6 +16,7 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-rng.h"
 #include "sysemu/rng.h"
+#include "sysemu/rng-random.h"
 #include "qom/object_interfaces.h"
 #include "trace.h"
 
@@ -189,27 +190,24 @@ static void virtio_rng_device_realize(DeviceState *dev, Error **errp)
     }
 
     if (vrng->conf.rng == NULL) {
-        vrng->conf.default_backend = RNG_RANDOM(object_new(TYPE_RNG_RANDOM));
+        Object *default_backend = object_new(TYPE_RNG_RANDOM);
 
-        user_creatable_complete(USER_CREATABLE(vrng->conf.default_backend),
+        user_creatable_complete(USER_CREATABLE(default_backend),
                                 &local_err);
         if (local_err) {
             error_propagate(errp, local_err);
-            object_unref(OBJECT(vrng->conf.default_backend));
+            object_unref(default_backend);
             return;
         }
 
-        object_property_add_child(OBJECT(dev),
-                                  "default-backend",
-                                  OBJECT(vrng->conf.default_backend),
-                                  NULL);
+        object_property_add_child(OBJECT(dev), "default-backend",
+                                  default_backend, &error_abort);
 
         /* The child property took a reference, we can safely drop ours now */
-        object_unref(OBJECT(vrng->conf.default_backend));
+        object_unref(default_backend);
 
-        object_property_set_link(OBJECT(dev),
-                                 OBJECT(vrng->conf.default_backend),
-                                 "rng", NULL);
+        object_property_set_link(OBJECT(dev), default_backend,
+                                 "rng", &error_abort);
     }
 
     vrng->rng = vrng->conf.rng;
diff --git a/include/hw/virtio/virtio-rng.h b/include/hw/virtio/virtio-rng.h
index 922dce7caccf..28ff752c4096 100644
--- a/include/hw/virtio/virtio-rng.h
+++ b/include/hw/virtio/virtio-rng.h
@@ -13,7 +13,6 @@
 #define QEMU_VIRTIO_RNG_H
 
 #include "sysemu/rng.h"
-#include "sysemu/rng-random.h"
 #include "standard-headers/linux/virtio_rng.h"
 
 #define TYPE_VIRTIO_RNG "virtio-rng-device"
@@ -26,7 +25,6 @@ struct VirtIORNGConf {
     RngBackend *rng;
     uint64_t max_bytes;
     uint32_t period_ms;
-    RngRandom *default_backend;
 };
 
 typedef struct VirtIORNG {
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v7 4/4] virtio-rng: change default backend to rng-builtin
  2019-05-29 14:31 [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom() Laurent Vivier
                   ` (2 preceding siblings ...)
  2019-05-29 14:31 ` [Qemu-devel] [PATCH v7 3/4] virtio-rng: Keep the default backend out of VirtIORNGConf Laurent Vivier
@ 2019-05-29 14:31 ` Laurent Vivier
  2019-06-05 13:05 ` [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom() Markus Armbruster
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Laurent Vivier @ 2019-05-29 14:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Daniel P . Berrangé,
	Kashyap Chamarthy, Amit Shah, Richard Henderson,
	Richard W . M . Jones, Markus Armbruster, Michael S. Tsirkin

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 backends/rng-builtin.c | 1 -
 hw/virtio/virtio-rng.c | 3 +--
 include/sysemu/rng.h   | 2 ++
 qemu-options.hx        | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/backends/rng-builtin.c b/backends/rng-builtin.c
index 3381d47174df..ba1b8d66b83e 100644
--- a/backends/rng-builtin.c
+++ b/backends/rng-builtin.c
@@ -10,7 +10,6 @@
 #include "qemu/main-loop.h"
 #include "qemu/guest-random.h"
 
-#define TYPE_RNG_BUILTIN "rng-builtin"
 #define RNG_BUILTIN(obj) OBJECT_CHECK(RngBuiltin, (obj), TYPE_RNG_BUILTIN)
 
 typedef struct RngBuiltin {
diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index 73ffb476e030..55745f75a1c9 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -16,7 +16,6 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-rng.h"
 #include "sysemu/rng.h"
-#include "sysemu/rng-random.h"
 #include "qom/object_interfaces.h"
 #include "trace.h"
 
@@ -190,7 +189,7 @@ static void virtio_rng_device_realize(DeviceState *dev, Error **errp)
     }
 
     if (vrng->conf.rng == NULL) {
-        Object *default_backend = object_new(TYPE_RNG_RANDOM);
+        Object *default_backend = object_new(TYPE_RNG_BUILTIN);
 
         user_creatable_complete(USER_CREATABLE(default_backend),
                                 &local_err);
diff --git a/include/sysemu/rng.h b/include/sysemu/rng.h
index 27b37da05d2e..9dc5e0e90100 100644
--- a/include/sysemu/rng.h
+++ b/include/sysemu/rng.h
@@ -24,6 +24,8 @@
 #define RNG_BACKEND_CLASS(klass) \
     OBJECT_CLASS_CHECK(RngBackendClass, (klass), TYPE_RNG_BACKEND)
 
+#define TYPE_RNG_BUILTIN "rng-builtin"
+
 typedef struct RngRequest RngRequest;
 typedef struct RngBackendClass RngBackendClass;
 typedef struct RngBackend RngBackend;
diff --git a/qemu-options.hx b/qemu-options.hx
index 4e6a6828d7ce..7746fe9bed6a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4327,7 +4327,7 @@ The @option{share} boolean option is @var{on} by default with memfd.
 Creates a random number generator backend which obtains entropy from
 QEMU builtin functions. The @option{id} parameter is a unique ID that
 will be used to reference this entropy backend from the @option{virtio-rng}
-device.
+device. By default, the @option{virtio-rng} device uses this RNG backend.
 
 @item -object rng-random,id=@var{id},filename=@var{/dev/random}
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
  2019-05-29 14:31 [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom() Laurent Vivier
                   ` (3 preceding siblings ...)
  2019-05-29 14:31 ` [Qemu-devel] [PATCH v7 4/4] virtio-rng: change default backend to rng-builtin Laurent Vivier
@ 2019-06-05 13:05 ` Markus Armbruster
  2019-06-05 13:58   ` Laurent Vivier
  2019-06-11  8:42 ` Laurent Vivier
  2019-07-02 13:21 ` Michael S. Tsirkin
  6 siblings, 1 reply; 20+ messages in thread
From: Markus Armbruster @ 2019-06-05 13:05 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: Daniel P . Berrangé,
	Amit Shah, Kashyap Chamarthy, Richard Henderson,
	Richard W . M . Jones, qemu-devel, Michael S. Tsirkin

Laurent Vivier <lvivier@redhat.com> writes:

> Add a new RNG backend using QEMU builtin getrandom function.
>
> v7: rebase on master
>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)

Pardon the ignorant question: why is that necessary?


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
  2019-06-05 13:05 ` [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom() Markus Armbruster
@ 2019-06-05 13:58   ` Laurent Vivier
  2019-06-05 17:56     ` Markus Armbruster
  0 siblings, 1 reply; 20+ messages in thread
From: Laurent Vivier @ 2019-06-05 13:58 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Daniel P.Berrangé,
	Amit Shah, Kashyap Chamarthy, Richard Henderson,
	Richard W . M . Jones, qemu-devel, Michael S. Tsirkin

On 05/06/2019 15:05, Markus Armbruster wrote:
> Laurent Vivier <lvivier@redhat.com> writes:
> 
>> Add a new RNG backend using QEMU builtin getrandom function.
>>
>> v7: rebase on master
>>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)
> 
> Pardon the ignorant question: why is that necessary?
> 

Because request_entropy() function is called while the request is not in
the requests queue, so the loop on !QSIMPLEQ_EMPTY(&s->parent.requests)
doens't process it. The request is added just after the call.

thanks,
Laurent






^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
  2019-06-05 13:58   ` Laurent Vivier
@ 2019-06-05 17:56     ` Markus Armbruster
  2019-06-05 18:36       ` Laurent Vivier
  0 siblings, 1 reply; 20+ messages in thread
From: Markus Armbruster @ 2019-06-05 17:56 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: Daniel P.Berrangé,
	Kashyap Chamarthy, Amit Shah, Richard Henderson, qemu-devel,
	Richard W . M . Jones, Michael S. Tsirkin

Laurent Vivier <lvivier@redhat.com> writes:

> On 05/06/2019 15:05, Markus Armbruster wrote:
>> Laurent Vivier <lvivier@redhat.com> writes:
>> 
>>> Add a new RNG backend using QEMU builtin getrandom function.
>>>
>>> v7: rebase on master
>>>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)
>> 
>> Pardon the ignorant question: why is that necessary?
>> 
>
> Because request_entropy() function is called while the request is not in
> the requests queue, so the loop on !QSIMPLEQ_EMPTY(&s->parent.requests)
> doens't process it. The request is added just after the call.

In rng_backend_request_entropy().  I see.  Any particular reason for
this order?  "I don't know" is an acceptable answer :)


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
  2019-06-05 17:56     ` Markus Armbruster
@ 2019-06-05 18:36       ` Laurent Vivier
  2019-06-06  4:45         ` Markus Armbruster
  0 siblings, 1 reply; 20+ messages in thread
From: Laurent Vivier @ 2019-06-05 18:36 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Daniel P.Berrangé,
	Kashyap Chamarthy, Amit Shah, Richard Henderson, qemu-devel,
	Richard W . M . Jones, Michael S. Tsirkin

On 05/06/2019 19:56, Markus Armbruster wrote:
> Laurent Vivier <lvivier@redhat.com> writes:
> 
>> On 05/06/2019 15:05, Markus Armbruster wrote:
>>> Laurent Vivier <lvivier@redhat.com> writes:
>>>
>>>> Add a new RNG backend using QEMU builtin getrandom function.
>>>>
>>>> v7: rebase on master
>>>>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)
>>>
>>> Pardon the ignorant question: why is that necessary?
>>>
>>
>> Because request_entropy() function is called while the request is not in
>> the requests queue, so the loop on !QSIMPLEQ_EMPTY(&s->parent.requests)
>> doens't process it. The request is added just after the call.
> 
> In rng_backend_request_entropy().  I see.  Any particular reason for
> this order?  "I don't know" is an acceptable answer :)
> 

Yes...

and there is a reason:

in rng_random_request_entropy(), QSIMPLEQ_EMPTY() is used to know if we
have to register an fd handler with qemu_set_fd_handler().

For me, it seemed easier to use QEMUBH rather than to change the
existing algorithm, as the backend has been thought to be asynchronous.

Thanks,
Laurent





^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
  2019-06-05 18:36       ` Laurent Vivier
@ 2019-06-06  4:45         ` Markus Armbruster
  0 siblings, 0 replies; 20+ messages in thread
From: Markus Armbruster @ 2019-06-06  4:45 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: Daniel P.Berrangé,
	Amit Shah, Kashyap Chamarthy, Richard Henderson, qemu-devel,
	Markus Armbruster, Richard W . M . Jones, Michael S. Tsirkin

Laurent Vivier <lvivier@redhat.com> writes:

> On 05/06/2019 19:56, Markus Armbruster wrote:
>> Laurent Vivier <lvivier@redhat.com> writes:
>> 
>>> On 05/06/2019 15:05, Markus Armbruster wrote:
>>>> Laurent Vivier <lvivier@redhat.com> writes:
>>>>
>>>>> Add a new RNG backend using QEMU builtin getrandom function.
>>>>>
>>>>> v7: rebase on master
>>>>>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)
>>>>
>>>> Pardon the ignorant question: why is that necessary?
>>>>
>>>
>>> Because request_entropy() function is called while the request is not in
>>> the requests queue, so the loop on !QSIMPLEQ_EMPTY(&s->parent.requests)
>>> doens't process it. The request is added just after the call.
>> 
>> In rng_backend_request_entropy().  I see.  Any particular reason for
>> this order?  "I don't know" is an acceptable answer :)
>> 
>
> Yes...
>
> and there is a reason:
>
> in rng_random_request_entropy(), QSIMPLEQ_EMPTY() is used to know if we
> have to register an fd handler with qemu_set_fd_handler().
>
> For me, it seemed easier to use QEMUBH rather than to change the
> existing algorithm, as the backend has been thought to be asynchronous.

In your shoes, I'd be tempted to explore whether changing the order
simplifies things overall.  I'm not asking you to do that; your patch is
okay as is.

Thanks!


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v7 2/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
  2019-05-29 14:31 ` [Qemu-devel] [PATCH v7 2/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom() Laurent Vivier
@ 2019-06-06  4:46   ` Markus Armbruster
  0 siblings, 0 replies; 20+ messages in thread
From: Markus Armbruster @ 2019-06-06  4:46 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: Daniel P . Berrangé,
	Amit Shah, Markus Armbruster, Kashyap Chamarthy,
	Richard Henderson, Richard W . M . Jones, qemu-devel,
	Michael S. Tsirkin

Laurent Vivier <lvivier@redhat.com> writes:

> Add a new RNG backend using QEMU builtin getrandom function.
>
> It can be created and used with something like:
>
>     ... -object rng-builtin,id=rng0 -device virtio-rng,rng=rng0 ...
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  backends/Makefile.objs |  2 +-
>  backends/rng-builtin.c | 78 ++++++++++++++++++++++++++++++++++++++++++
>  qemu-options.hx        |  7 ++++
>  3 files changed, 86 insertions(+), 1 deletion(-)
>  create mode 100644 backends/rng-builtin.c
>
> diff --git a/backends/Makefile.objs b/backends/Makefile.objs
> index 981e8e122f2c..f0691116e86e 100644
> --- a/backends/Makefile.objs
> +++ b/backends/Makefile.objs
> @@ -1,4 +1,4 @@
> -common-obj-y += rng.o rng-egd.o
> +common-obj-y += rng.o rng-egd.o rng-builtin.o
>  common-obj-$(CONFIG_POSIX) += rng-random.o
>  
>  common-obj-$(CONFIG_TPM) += tpm.o
> diff --git a/backends/rng-builtin.c b/backends/rng-builtin.c
> new file mode 100644
> index 000000000000..3381d47174df
> --- /dev/null
> +++ b/backends/rng-builtin.c
> @@ -0,0 +1,78 @@
> +/*
> + * QEMU Builtin Random Number Generator Backend
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "sysemu/rng.h"
> +#include "qemu/main-loop.h"
> +#include "qemu/guest-random.h"
> +
> +#define TYPE_RNG_BUILTIN "rng-builtin"
> +#define RNG_BUILTIN(obj) OBJECT_CHECK(RngBuiltin, (obj), TYPE_RNG_BUILTIN)
> +
> +typedef struct RngBuiltin {
> +    RngBackend parent;
> +    QEMUBH *bh;
> +} RngBuiltin;
> +
> +static void rng_builtin_receive_entropy_bh(void *opaque)
> +{
> +    RngBuiltin *s = opaque;
> +
> +    while (!QSIMPLEQ_EMPTY(&s->parent.requests)) {
> +        RngRequest *req = QSIMPLEQ_FIRST(&s->parent.requests);
> +
> +        qemu_guest_getrandom_nofail(req->data, req->size);
> +
> +        req->receive_entropy(req->opaque, req->data, req->size);
> +
> +        rng_backend_finalize_request(&s->parent, req);
> +    }
> +}
> +
> +static void rng_builtin_request_entropy(RngBackend *b, RngRequest *req)
> +{
> +    RngBuiltin *s = RNG_BUILTIN(b);
> +
> +    qemu_bh_schedule(s->bh);
> +}

A comment explaining the need for a BH would be nice.

Regardless:
Reviewed-by: Markus Armbruster <armbru@redhat.com>

[...]


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
  2019-05-29 14:31 [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom() Laurent Vivier
                   ` (4 preceding siblings ...)
  2019-06-05 13:05 ` [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom() Markus Armbruster
@ 2019-06-11  8:42 ` Laurent Vivier
  2019-06-17 12:09   ` Laurent Vivier
  2019-07-02 13:21 ` Michael S. Tsirkin
  6 siblings, 1 reply; 20+ messages in thread
From: Laurent Vivier @ 2019-06-11  8:42 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Daniel P . Berrangé,
	Amit Shah, Markus Armbruster, Kashyap Chamarthy,
	Richard Henderson, Richard W . M . Jones, qemu-devel

Michael,

Could you pick this series in the next virtio pull request?

If you disagree with some of my patches, could you take at least the
first one (from Kashyap)?

Thanks,
Laurent

On 29/05/2019 16:31, Laurent Vivier wrote:
> Add a new RNG backend using QEMU builtin getrandom function.
> 
> v7: rebase on master
>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)
> 
> v6: remove "sysemu/rng-random.h" from virtio-rng.c
>     rebase on qemu_getrandom v8
> 
> v5: PATCH 1 s/linux/Linux/
>     remove superfluous includes from rng-builtin.c
>     don't update rng-random documentation
>     add a patch from Markus to keep the default backend out of VirtIORNGConf
>     move TYPE_RNG_BUILTIN to sysemu/rng.h and remove sysemu/rng-builtin.h
> 
> v4: update PATCH 1 commit message
> 
> v3: Include Kashyap's patch in the series
>     Add a patch to change virtio-rng default backend to rng-builtin
> 
> v2: Update qemu-options.hx
>     describe the new backend and specify virtio-rng uses the
>     rng-random by default
> 
> Kashyap Chamarthy (1):
>   VirtIO-RNG: Update default entropy source to `/dev/urandom`
> 
> Laurent Vivier (2):
>   rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
>   virtio-rng: change default backend to rng-builtin
> 
> Markus Armbruster (1):
>   virtio-rng: Keep the default backend out of VirtIORNGConf
> 
>  backends/Makefile.objs         |  2 +-
>  backends/rng-builtin.c         | 77 ++++++++++++++++++++++++++++++++++
>  backends/rng-random.c          |  2 +-
>  hw/virtio/virtio-rng.c         | 19 ++++-----
>  include/hw/virtio/virtio-rng.h |  2 -
>  include/sysemu/rng.h           |  2 +
>  qemu-options.hx                |  9 +++-
>  7 files changed, 97 insertions(+), 16 deletions(-)
>  create mode 100644 backends/rng-builtin.c
> 



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
  2019-06-11  8:42 ` Laurent Vivier
@ 2019-06-17 12:09   ` Laurent Vivier
  0 siblings, 0 replies; 20+ messages in thread
From: Laurent Vivier @ 2019-06-17 12:09 UTC (permalink / raw)
  To: Amit Shah, Amit Shah
  Cc: Daniel P . Berrangé,
	Kashyap Chamarthy, qemu-devel, Michael S. Tsirkin,
	Richard Henderson, Richard W . M . Jones, Markus Armbruster

On 11/06/2019 10:42, Laurent Vivier wrote:
> Michael,
> 
> Could you pick this series in the next virtio pull request?

Or perhaps Amit?

Thanks,
Laurent

> 
> If you disagree with some of my patches, could you take at least the
> first one (from Kashyap)?
> 
> Thanks,
> Laurent
> 
> On 29/05/2019 16:31, Laurent Vivier wrote:
>> Add a new RNG backend using QEMU builtin getrandom function.
>>
>> v7: rebase on master
>>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)
>>
>> v6: remove "sysemu/rng-random.h" from virtio-rng.c
>>     rebase on qemu_getrandom v8
>>
>> v5: PATCH 1 s/linux/Linux/
>>     remove superfluous includes from rng-builtin.c
>>     don't update rng-random documentation
>>     add a patch from Markus to keep the default backend out of VirtIORNGConf
>>     move TYPE_RNG_BUILTIN to sysemu/rng.h and remove sysemu/rng-builtin.h
>>
>> v4: update PATCH 1 commit message
>>
>> v3: Include Kashyap's patch in the series
>>     Add a patch to change virtio-rng default backend to rng-builtin
>>
>> v2: Update qemu-options.hx
>>     describe the new backend and specify virtio-rng uses the
>>     rng-random by default
>>
>> Kashyap Chamarthy (1):
>>   VirtIO-RNG: Update default entropy source to `/dev/urandom`
>>
>> Laurent Vivier (2):
>>   rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
>>   virtio-rng: change default backend to rng-builtin
>>
>> Markus Armbruster (1):
>>   virtio-rng: Keep the default backend out of VirtIORNGConf
>>
>>  backends/Makefile.objs         |  2 +-
>>  backends/rng-builtin.c         | 77 ++++++++++++++++++++++++++++++++++
>>  backends/rng-random.c          |  2 +-
>>  hw/virtio/virtio-rng.c         | 19 ++++-----
>>  include/hw/virtio/virtio-rng.h |  2 -
>>  include/sysemu/rng.h           |  2 +
>>  qemu-options.hx                |  9 +++-
>>  7 files changed, 97 insertions(+), 16 deletions(-)
>>  create mode 100644 backends/rng-builtin.c
>>
> 
> 



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v7 1/4] VirtIO-RNG: Update default entropy source to `/dev/urandom`
  2019-05-29 14:31 ` [Qemu-devel] [PATCH v7 1/4] VirtIO-RNG: Update default entropy source to `/dev/urandom` Laurent Vivier
@ 2019-06-26 20:29   ` Laurent Vivier
  2019-07-02  7:41   ` Laurent Vivier
  2019-07-03 14:26   ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 20+ messages in thread
From: Laurent Vivier @ 2019-06-26 20:29 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P . Berrangé,
	Amit Shah, Kashyap Chamarthy, Richard Henderson,
	Richard W . M . Jones, Markus Armbruster, Michael S. Tsirkin,
	Stefan Hajnoczi

On 29/05/2019 16:31, Laurent Vivier wrote:
> From: Kashyap Chamarthy <kchamart@redhat.com>
> 
> When QEMU exposes a VirtIO-RNG device to the guest, that device needs a
> source of entropy, and that source needs to be "non-blocking", like
> `/dev/urandom`.  However, currently QEMU defaults to the problematic
> `/dev/random`, which on Linux is "blocking" (as in, it waits until
> sufficient entropy is available).
> 
> Why prefer `/dev/urandom` over `/dev/random`?
> ---------------------------------------------
> 
> The man pages of urandom(4) and random(4) state:
> 
>     "The /dev/random device is a legacy interface which dates back to a
>     time where the cryptographic primitives used in the implementation
>     of /dev/urandom were not widely trusted.  It will return random
>     bytes only within the estimated number of bits of fresh noise in the
>     entropy pool, blocking if necessary.  /dev/random is suitable for
>     applications that need high quality randomness, and can afford
>     indeterminate delays."
> 
> Further, the "Usage" section of the said man pages state:
> 
>     "The /dev/random interface is considered a legacy interface, and
>     /dev/urandom is preferred and sufficient in all use cases, with the
>     exception of applications which require randomness during early boot
>     time; for these applications, getrandom(2) must be used instead,
>     because it will block until the entropy pool is initialized.
> 
>     "If a seed file is saved across reboots as recommended below (all
>     major Linux distributions have done this since 2000 at least), the
>     output is cryptographically secure against attackers without local
>     root access as soon as it is reloaded in the boot sequence, and
>     perfectly adequate for network encryption session keys.  Since reads
>     from /dev/random may block, users will usually want to open it in
>     nonblocking mode (or perform a read with timeout), and provide some
>     sort of user notification if the desired entropy is not immediately
>     available."
> 
> And refer to random(7) for a comparison of `/dev/random` and
> `/dev/urandom`.
> 
> What about other OSes?
> ----------------------
> 
> `/dev/urandom` exists and works on OS-X, FreeBSD, DragonFlyBSD, NetBSD
> and OpenBSD, which cover all the non-Linux platforms we explicitly
> support, aside from Windows.
> 
> On Windows `/dev/random` doesn't work either so we don't regress.
> This is actually another argument in favour of using the newly
> proposed 'rng-builtin' backend by default, as that will work on
> Windows.
> 
>     - - -
> 
> Given the above, change the entropy source for VirtIO-RNG device to
> `/dev/urandom`.
> 
> Related discussion in these[1][2] past threads.
> 
> [1] https://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg08335.html
>     -- "RNG: Any reason QEMU doesn't default to `/dev/urandom`?"
> [2] https://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg02724.html
>     -- "[RFC] Virtio RNG: Consider changing the default entropy source to
>        /dev/urandom"
> 
> Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  backends/rng-random.c | 2 +-
>  qemu-options.hx       | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/backends/rng-random.c b/backends/rng-random.c
> index e2a49b0571d7..eff36ef14084 100644
> --- a/backends/rng-random.c
> +++ b/backends/rng-random.c
> @@ -112,7 +112,7 @@ static void rng_random_init(Object *obj)
>                              rng_random_set_filename,
>                              NULL);
>  
> -    s->filename = g_strdup("/dev/random");
> +    s->filename = g_strdup("/dev/urandom");
>      s->fd = -1;
>  }
>  
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 39dc17042967..f6e9bd1d9c42 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4328,7 +4328,7 @@ Creates a random number generator backend which obtains entropy from
>  a device on the host. The @option{id} parameter is a unique ID that
>  will be used to reference this entropy backend from the @option{virtio-rng}
>  device. The @option{filename} parameter specifies which file to obtain
> -entropy from and if omitted defaults to @option{/dev/random}.
> +entropy from and if omitted defaults to @option{/dev/urandom}.
>  
>  @item -object rng-egd,id=@var{id},chardev=@var{chardevid}
>  
> 

I can push this one via a trivial-branch pull request if no one thinks
other patches of the series are ready to be merged.

Thanks,
Laurent


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v7 1/4] VirtIO-RNG: Update default entropy source to `/dev/urandom`
  2019-05-29 14:31 ` [Qemu-devel] [PATCH v7 1/4] VirtIO-RNG: Update default entropy source to `/dev/urandom` Laurent Vivier
  2019-06-26 20:29   ` Laurent Vivier
@ 2019-07-02  7:41   ` Laurent Vivier
  2019-07-03 14:26   ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 20+ messages in thread
From: Laurent Vivier @ 2019-07-02  7:41 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel
  Cc: Daniel P . Berrangé,
	Amit Shah, qemu-trivial, Kashyap Chamarthy, Richard Henderson,
	Richard W . M . Jones, Markus Armbruster, Michael S. Tsirkin,
	Stefan Hajnoczi

Le 29/05/2019 à 16:31, Laurent Vivier a écrit :
> From: Kashyap Chamarthy <kchamart@redhat.com>
> 
> When QEMU exposes a VirtIO-RNG device to the guest, that device needs a
> source of entropy, and that source needs to be "non-blocking", like
> `/dev/urandom`.  However, currently QEMU defaults to the problematic
> `/dev/random`, which on Linux is "blocking" (as in, it waits until
> sufficient entropy is available).
> 
> Why prefer `/dev/urandom` over `/dev/random`?
> ---------------------------------------------
> 
> The man pages of urandom(4) and random(4) state:
> 
>     "The /dev/random device is a legacy interface which dates back to a
>     time where the cryptographic primitives used in the implementation
>     of /dev/urandom were not widely trusted.  It will return random
>     bytes only within the estimated number of bits of fresh noise in the
>     entropy pool, blocking if necessary.  /dev/random is suitable for
>     applications that need high quality randomness, and can afford
>     indeterminate delays."
> 
> Further, the "Usage" section of the said man pages state:
> 
>     "The /dev/random interface is considered a legacy interface, and
>     /dev/urandom is preferred and sufficient in all use cases, with the
>     exception of applications which require randomness during early boot
>     time; for these applications, getrandom(2) must be used instead,
>     because it will block until the entropy pool is initialized.
> 
>     "If a seed file is saved across reboots as recommended below (all
>     major Linux distributions have done this since 2000 at least), the
>     output is cryptographically secure against attackers without local
>     root access as soon as it is reloaded in the boot sequence, and
>     perfectly adequate for network encryption session keys.  Since reads
>     from /dev/random may block, users will usually want to open it in
>     nonblocking mode (or perform a read with timeout), and provide some
>     sort of user notification if the desired entropy is not immediately
>     available."
> 
> And refer to random(7) for a comparison of `/dev/random` and
> `/dev/urandom`.
> 
> What about other OSes?
> ----------------------
> 
> `/dev/urandom` exists and works on OS-X, FreeBSD, DragonFlyBSD, NetBSD
> and OpenBSD, which cover all the non-Linux platforms we explicitly
> support, aside from Windows.
> 
> On Windows `/dev/random` doesn't work either so we don't regress.
> This is actually another argument in favour of using the newly
> proposed 'rng-builtin' backend by default, as that will work on
> Windows.
> 
>     - - -
> 
> Given the above, change the entropy source for VirtIO-RNG device to
> `/dev/urandom`.
> 
> Related discussion in these[1][2] past threads.
> 
> [1] https://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg08335.html
>     -- "RNG: Any reason QEMU doesn't default to `/dev/urandom`?"
> [2] https://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg02724.html
>     -- "[RFC] Virtio RNG: Consider changing the default entropy source to
>        /dev/urandom"
> 
> Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  backends/rng-random.c | 2 +-
>  qemu-options.hx       | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/backends/rng-random.c b/backends/rng-random.c
> index e2a49b0571d7..eff36ef14084 100644
> --- a/backends/rng-random.c
> +++ b/backends/rng-random.c
> @@ -112,7 +112,7 @@ static void rng_random_init(Object *obj)
>                              rng_random_set_filename,
>                              NULL);
>  
> -    s->filename = g_strdup("/dev/random");
> +    s->filename = g_strdup("/dev/urandom");
>      s->fd = -1;
>  }
>  
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 39dc17042967..f6e9bd1d9c42 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4328,7 +4328,7 @@ Creates a random number generator backend which obtains entropy from
>  a device on the host. The @option{id} parameter is a unique ID that
>  will be used to reference this entropy backend from the @option{virtio-rng}
>  device. The @option{filename} parameter specifies which file to obtain
> -entropy from and if omitted defaults to @option{/dev/random}.
> +entropy from and if omitted defaults to @option{/dev/urandom}.
>  
>  @item -object rng-egd,id=@var{id},chardev=@var{chardevid}
>  
> 

As this patch is trivial and reviewed by at least 3 persons, and it
seems no one wants to take care of it, I will push it through my next
trivial pull request.

Thanks,
Laurent


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
  2019-05-29 14:31 [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom() Laurent Vivier
                   ` (5 preceding siblings ...)
  2019-06-11  8:42 ` Laurent Vivier
@ 2019-07-02 13:21 ` Michael S. Tsirkin
  2019-07-02 16:48   ` Laurent Vivier
  6 siblings, 1 reply; 20+ messages in thread
From: Michael S. Tsirkin @ 2019-07-02 13:21 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: Daniel P . Berrangé,
	Kashyap Chamarthy, qemu-devel, Amit Shah, Richard Henderson,
	Markus Armbruster, Richard W . M . Jones

On Wed, May 29, 2019 at 04:31:02PM +0200, Laurent Vivier wrote:
> Add a new RNG backend using QEMU builtin getrandom function.
> 
> v7: rebase on master
>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)
> 
> v6: remove "sysemu/rng-random.h" from virtio-rng.c
>     rebase on qemu_getrandom v8
> 
> v5: PATCH 1 s/linux/Linux/
>     remove superfluous includes from rng-builtin.c
>     don't update rng-random documentation
>     add a patch from Markus to keep the default backend out of VirtIORNGConf
>     move TYPE_RNG_BUILTIN to sysemu/rng.h and remove sysemu/rng-builtin.h
> 
> v4: update PATCH 1 commit message
> 
> v3: Include Kashyap's patch in the series
>     Add a patch to change virtio-rng default backend to rng-builtin
> 
> v2: Update qemu-options.hx
>     describe the new backend and specify virtio-rng uses the
>     rng-random by default


Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

feel free to merge.

> Kashyap Chamarthy (1):
>   VirtIO-RNG: Update default entropy source to `/dev/urandom`
> 
> Laurent Vivier (2):
>   rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
>   virtio-rng: change default backend to rng-builtin
> 
> Markus Armbruster (1):
>   virtio-rng: Keep the default backend out of VirtIORNGConf
> 
>  backends/Makefile.objs         |  2 +-
>  backends/rng-builtin.c         | 77 ++++++++++++++++++++++++++++++++++
>  backends/rng-random.c          |  2 +-
>  hw/virtio/virtio-rng.c         | 19 ++++-----
>  include/hw/virtio/virtio-rng.h |  2 -
>  include/sysemu/rng.h           |  2 +
>  qemu-options.hx                |  9 +++-
>  7 files changed, 97 insertions(+), 16 deletions(-)
>  create mode 100644 backends/rng-builtin.c
> 
> -- 
> 2.20.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
  2019-07-02 13:21 ` Michael S. Tsirkin
@ 2019-07-02 16:48   ` Laurent Vivier
  2019-07-03 14:23     ` Amit Shah
  0 siblings, 1 reply; 20+ messages in thread
From: Laurent Vivier @ 2019-07-02 16:48 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Daniel P. Berrangé,
	Kashyap Chamarthy, qemu-devel, Amit Shah, Richard Henderson,
	Markus Armbruster, Richard W . M . Jones

On 02/07/2019 15:21, Michael S. Tsirkin wrote:
> On Wed, May 29, 2019 at 04:31:02PM +0200, Laurent Vivier wrote:
>> Add a new RNG backend using QEMU builtin getrandom function.
>>
>> v7: rebase on master
>>     Make rng-builtin asynchronous with QEMUBH (removed existing R-b)
>>
>> v6: remove "sysemu/rng-random.h" from virtio-rng.c
>>     rebase on qemu_getrandom v8
>>
>> v5: PATCH 1 s/linux/Linux/
>>     remove superfluous includes from rng-builtin.c
>>     don't update rng-random documentation
>>     add a patch from Markus to keep the default backend out of VirtIORNGConf
>>     move TYPE_RNG_BUILTIN to sysemu/rng.h and remove sysemu/rng-builtin.h
>>
>> v4: update PATCH 1 commit message
>>
>> v3: Include Kashyap's patch in the series
>>     Add a patch to change virtio-rng default backend to rng-builtin
>>
>> v2: Update qemu-options.hx
>>     describe the new backend and specify virtio-rng uses the
>>     rng-random by default
> 
> 
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> 
> feel free to merge.

Thank you Michael.

I've already included PATCH 1 in a pull-request for trivial patches branch.

I'm not sure the other patches are good candidates for trivial patches
branch, but is there any other maintainer that can include them in a
pull request (before the freeze)?

Amit?
[Do you want I manage a virtio-rng pull-request for you?]

Thanks,
Laurent


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
  2019-07-02 16:48   ` Laurent Vivier
@ 2019-07-03 14:23     ` Amit Shah
  2019-07-03 14:57       ` Laurent Vivier
  0 siblings, 1 reply; 20+ messages in thread
From: Amit Shah @ 2019-07-03 14:23 UTC (permalink / raw)
  To: Laurent Vivier, Michael S. Tsirkin
  Cc: Daniel P. Berrangé,
	Kashyap Chamarthy, qemu-devel, Amit Shah, Richard Henderson,
	Markus Armbruster, Richard W . M . Jones

On Tue, 2019-07-02 at 18:48 +0200, Laurent Vivier wrote:
> On 02/07/2019 15:21, Michael S. Tsirkin wrote:
> > On Wed, May 29, 2019 at 04:31:02PM +0200, Laurent Vivier wrote:
> > > Add a new RNG backend using QEMU builtin getrandom function.
> > > 
> > > v7: rebase on master
> > >     Make rng-builtin asynchronous with QEMUBH (removed existing
> > > R-b)
> > > 
> > > v6: remove "sysemu/rng-random.h" from virtio-rng.c
> > >     rebase on qemu_getrandom v8
> > > 
> > > v5: PATCH 1 s/linux/Linux/
> > >     remove superfluous includes from rng-builtin.c
> > >     don't update rng-random documentation
> > >     add a patch from Markus to keep the default backend out of
> > > VirtIORNGConf
> > >     move TYPE_RNG_BUILTIN to sysemu/rng.h and remove sysemu/rng-
> > > builtin.h
> > > 
> > > v4: update PATCH 1 commit message
> > > 
> > > v3: Include Kashyap's patch in the series
> > >     Add a patch to change virtio-rng default backend to rng-
> > > builtin
> > > 
> > > v2: Update qemu-options.hx
> > >     describe the new backend and specify virtio-rng uses the
> > >     rng-random by default
> > 
> > 
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > feel free to merge.
> 
> Thank you Michael.
> 
> I've already included PATCH 1 in a pull-request for trivial patches
> branch.
> 
> I'm not sure the other patches are good candidates for trivial
> patches
> branch, but is there any other maintainer that can include them in a
> pull request (before the freeze)?
> 
> Amit?
> [Do you want I manage a virtio-rng pull-request for you?]

Hello Laurent,

Apologies as I haven't been around for a bit.

I don't mind you doing the pull req yourself if you have sufficient
reviews.  Do you also want to consider maintaining rng yourself?

Thanks,




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v7 1/4] VirtIO-RNG: Update default entropy source to `/dev/urandom`
  2019-05-29 14:31 ` [Qemu-devel] [PATCH v7 1/4] VirtIO-RNG: Update default entropy source to `/dev/urandom` Laurent Vivier
  2019-06-26 20:29   ` Laurent Vivier
  2019-07-02  7:41   ` Laurent Vivier
@ 2019-07-03 14:26   ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-03 14:26 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel
  Cc: Daniel P . Berrangé,
	Amit Shah, Kashyap Chamarthy, Richard Henderson,
	Richard W . M . Jones, Markus Armbruster, Michael S. Tsirkin,
	Stefan Hajnoczi

On 5/29/19 4:31 PM, Laurent Vivier wrote:
> From: Kashyap Chamarthy <kchamart@redhat.com>
> 
> When QEMU exposes a VirtIO-RNG device to the guest, that device needs a
> source of entropy, and that source needs to be "non-blocking", like
> `/dev/urandom`.  However, currently QEMU defaults to the problematic
> `/dev/random`, which on Linux is "blocking" (as in, it waits until
> sufficient entropy is available).
> 
> Why prefer `/dev/urandom` over `/dev/random`?
> ---------------------------------------------
> 
> The man pages of urandom(4) and random(4) state:
> 
>     "The /dev/random device is a legacy interface which dates back to a
>     time where the cryptographic primitives used in the implementation
>     of /dev/urandom were not widely trusted.  It will return random
>     bytes only within the estimated number of bits of fresh noise in the
>     entropy pool, blocking if necessary.  /dev/random is suitable for
>     applications that need high quality randomness, and can afford
>     indeterminate delays."
> 
> Further, the "Usage" section of the said man pages state:
> 
>     "The /dev/random interface is considered a legacy interface, and
>     /dev/urandom is preferred and sufficient in all use cases, with the
>     exception of applications which require randomness during early boot
>     time; for these applications, getrandom(2) must be used instead,
>     because it will block until the entropy pool is initialized.
> 
>     "If a seed file is saved across reboots as recommended below (all
>     major Linux distributions have done this since 2000 at least), the
>     output is cryptographically secure against attackers without local
>     root access as soon as it is reloaded in the boot sequence, and
>     perfectly adequate for network encryption session keys.  Since reads
>     from /dev/random may block, users will usually want to open it in
>     nonblocking mode (or perform a read with timeout), and provide some
>     sort of user notification if the desired entropy is not immediately
>     available."
> 
> And refer to random(7) for a comparison of `/dev/random` and
> `/dev/urandom`.
> 
> What about other OSes?
> ----------------------
> 
> `/dev/urandom` exists and works on OS-X, FreeBSD, DragonFlyBSD, NetBSD
> and OpenBSD, which cover all the non-Linux platforms we explicitly
> support, aside from Windows.
> 
> On Windows `/dev/random` doesn't work either so we don't regress.
> This is actually another argument in favour of using the newly
> proposed 'rng-builtin' backend by default, as that will work on
> Windows.
> 
>     - - -
> 
> Given the above, change the entropy source for VirtIO-RNG device to
> `/dev/urandom`.
> 
> Related discussion in these[1][2] past threads.
> 
> [1] https://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg08335.html
>     -- "RNG: Any reason QEMU doesn't default to `/dev/urandom`?"
> [2] https://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg02724.html
>     -- "[RFC] Virtio RNG: Consider changing the default entropy source to
>        /dev/urandom"
> 
> Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  backends/rng-random.c | 2 +-
>  qemu-options.hx       | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/backends/rng-random.c b/backends/rng-random.c
> index e2a49b0571d7..eff36ef14084 100644
> --- a/backends/rng-random.c
> +++ b/backends/rng-random.c
> @@ -112,7 +112,7 @@ static void rng_random_init(Object *obj)
>                              rng_random_set_filename,
>                              NULL);
>  
> -    s->filename = g_strdup("/dev/random");
> +    s->filename = g_strdup("/dev/urandom");
>      s->fd = -1;
>  }
>  
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 39dc17042967..f6e9bd1d9c42 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4328,7 +4328,7 @@ Creates a random number generator backend which obtains entropy from
>  a device on the host. The @option{id} parameter is a unique ID that
>  will be used to reference this entropy backend from the @option{virtio-rng}
>  device. The @option{filename} parameter specifies which file to obtain
> -entropy from and if omitted defaults to @option{/dev/random}.
> +entropy from and if omitted defaults to @option{/dev/urandom}.
>  
>  @item -object rng-egd,id=@var{id},chardev=@var{chardevid}
>  
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
  2019-07-03 14:23     ` Amit Shah
@ 2019-07-03 14:57       ` Laurent Vivier
  0 siblings, 0 replies; 20+ messages in thread
From: Laurent Vivier @ 2019-07-03 14:57 UTC (permalink / raw)
  To: Amit Shah, Michael S. Tsirkin
  Cc: Daniel P. Berrangé,
	Kashyap Chamarthy, qemu-devel, Amit Shah, Richard Henderson,
	Markus Armbruster, Richard W . M . Jones

On 03/07/2019 16:23, Amit Shah wrote:
> On Tue, 2019-07-02 at 18:48 +0200, Laurent Vivier wrote:
>> On 02/07/2019 15:21, Michael S. Tsirkin wrote:
>>> On Wed, May 29, 2019 at 04:31:02PM +0200, Laurent Vivier wrote:
>>>> Add a new RNG backend using QEMU builtin getrandom function.
>>>>
>>>> v7: rebase on master
>>>>     Make rng-builtin asynchronous with QEMUBH (removed existing
>>>> R-b)
>>>>
>>>> v6: remove "sysemu/rng-random.h" from virtio-rng.c
>>>>     rebase on qemu_getrandom v8
>>>>
>>>> v5: PATCH 1 s/linux/Linux/
>>>>     remove superfluous includes from rng-builtin.c
>>>>     don't update rng-random documentation
>>>>     add a patch from Markus to keep the default backend out of
>>>> VirtIORNGConf
>>>>     move TYPE_RNG_BUILTIN to sysemu/rng.h and remove sysemu/rng-
>>>> builtin.h
>>>>
>>>> v4: update PATCH 1 commit message
>>>>
>>>> v3: Include Kashyap's patch in the series
>>>>     Add a patch to change virtio-rng default backend to rng-
>>>> builtin
>>>>
>>>> v2: Update qemu-options.hx
>>>>     describe the new backend and specify virtio-rng uses the
>>>>     rng-random by default
>>>
>>>
>>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>>>
>>> feel free to merge.
>>
>> Thank you Michael.
>>
>> I've already included PATCH 1 in a pull-request for trivial patches
>> branch.
>>
>> I'm not sure the other patches are good candidates for trivial
>> patches
>> branch, but is there any other maintainer that can include them in a
>> pull request (before the freeze)?
>>
>> Amit?
>> [Do you want I manage a virtio-rng pull-request for you?]
> 
> Hello Laurent,
> 
> Apologies as I haven't been around for a bit.

No problem.

> I don't mind you doing the pull req yourself if you have sufficient
> reviews.

I think it's too late to push this in 4.1., so it this will wait next
release.

>   Do you also want to consider maintaining rng yourself?

It's up to you: if you think you don't have time to maintain virtio-rng,
I can manage this.

Thanks,
Laurent


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2019-07-03 14:58 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-29 14:31 [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom() Laurent Vivier
2019-05-29 14:31 ` [Qemu-devel] [PATCH v7 1/4] VirtIO-RNG: Update default entropy source to `/dev/urandom` Laurent Vivier
2019-06-26 20:29   ` Laurent Vivier
2019-07-02  7:41   ` Laurent Vivier
2019-07-03 14:26   ` Philippe Mathieu-Daudé
2019-05-29 14:31 ` [Qemu-devel] [PATCH v7 2/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom() Laurent Vivier
2019-06-06  4:46   ` Markus Armbruster
2019-05-29 14:31 ` [Qemu-devel] [PATCH v7 3/4] virtio-rng: Keep the default backend out of VirtIORNGConf Laurent Vivier
2019-05-29 14:31 ` [Qemu-devel] [PATCH v7 4/4] virtio-rng: change default backend to rng-builtin Laurent Vivier
2019-06-05 13:05 ` [Qemu-devel] [PATCH v7 0/4] rng-builtin: add an RNG backend that uses qemu_guest_getrandom() Markus Armbruster
2019-06-05 13:58   ` Laurent Vivier
2019-06-05 17:56     ` Markus Armbruster
2019-06-05 18:36       ` Laurent Vivier
2019-06-06  4:45         ` Markus Armbruster
2019-06-11  8:42 ` Laurent Vivier
2019-06-17 12:09   ` Laurent Vivier
2019-07-02 13:21 ` Michael S. Tsirkin
2019-07-02 16:48   ` Laurent Vivier
2019-07-03 14:23     ` Amit Shah
2019-07-03 14:57       ` Laurent Vivier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.