[PATCH v3 0/2] docs: show how to spawn qemu-storage-daemon with fd passing

[PATCH v3 0/2] docs: show how to spawn qemu-storage-daemon with fd passing

[Stefan Hajnoczi]

v3:
 * Explain how to detect launch errors and that the listen socket must be
   closed in the parent process in order for this to work [Daniel]

v2:
 * Use /var/run/qmp.sock instead of /tmp/qmp-$PID.sock to prevent security
   issues with world-writeable directories [Rich, Daniel]
 * Add Patch 2 to fix insecure examples in the documentation [Rich, Daniel]

Add an example of how to spawn qemu-storage-daemon with fd passing. This
approach eliminates the need to busy wait for the QMP, NBD, or vhost-user
socket to become available.

Stefan Hajnoczi (2):
  docs: show how to spawn qemu-storage-daemon with fd passing
  docs: replace insecure /tmp examples in qsd docs

 docs/tools/qemu-storage-daemon.rst | 49 +++++++++++++++++++++++++++---
 1 file changed, 44 insertions(+), 5 deletions(-)

-- 
2.29.2

[PATCH v3 1/2] docs: show how to spawn qemu-storage-daemon with fd passing

[Stefan Hajnoczi]

The QMP monitor, NBD server, and vhost-user-blk export all support file
descriptor passing. This is a useful technique because it allows the
parent process to spawn and wait for qemu-storage-daemon without busy
waiting, which may delay startup due to arbitrary sleep() calls.

This Python example is inspired by the test case written for libnbd by
Richard W.M. Jones <rjones@redhat.com>:
https://gitlab.com/nbdkit/libnbd/-/commit/89113f484effb0e6c322314ba75c1cbe07a04543

Thanks to Daniel P. Berrangé <berrange@redhat.com> for suggestions on
how to get this working. Now let's document it!

Reported-by: Richard W.M. Jones <rjones@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
v2:
 * Use /var/run/qmp.sock instead of /tmp/qmp-$PID.sock to prevent
   security issues with world-writeable directories [Rich, Daniel]
---
 docs/tools/qemu-storage-daemon.rst | 42 ++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/docs/tools/qemu-storage-daemon.rst b/docs/tools/qemu-storage-daemon.rst
index f63627eaf6..789a8e4a75 100644
--- a/docs/tools/qemu-storage-daemon.rst
+++ b/docs/tools/qemu-storage-daemon.rst
@@ -101,10 +101,12 @@ Standard options:
 
 .. option:: --nbd-server addr.type=inet,addr.host=<host>,addr.port=<port>[,tls-creds=<id>][,tls-authz=<id>][,max-connections=<n>]
   --nbd-server addr.type=unix,addr.path=<path>[,tls-creds=<id>][,tls-authz=<id>][,max-connections=<n>]
+  --nbd-server addr.type=fd,addr.str=<fd>[,tls-creds=<id>][,tls-authz=<id>][,max-connections=<n>]
 
   is a server for NBD exports. Both TCP and UNIX domain sockets are supported.
-  TLS encryption can be configured using ``--object`` tls-creds-* and authz-*
-  secrets (see below).
+  A listen socket can be provided via file descriptor passing (see Examples
+  below). TLS encryption can be configured using ``--object`` tls-creds-* and
+  authz-* secrets (see below).
 
   To configure an NBD server on UNIX domain socket path ``/tmp/nbd.sock``::
 
@@ -127,6 +129,42 @@ QMP commands::
       --chardev socket,path=qmp.sock,server,nowait,id=char1 \
       --monitor chardev=char1
 
+Launch the daemon from Python with a QMP monitor socket using file descriptor
+passing so there is no need to busy wait for the QMP monitor to become
+available::
+
+  #!/usr/bin/env python3
+  import subprocess
+  import socket
+
+  sock_path = '/var/run/qmp.sock'
+
+  with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as listen_sock:
+      listen_sock.bind(sock_path)
+      listen_sock.listen()
+
+      fd = listen_sock.fileno()
+
+      subprocess.Popen(
+          ['qemu-storage-daemon',
+           '--chardev', f'socket,fd={fd},server=on,id=char1',
+           '--monitor', 'chardev=char1'],
+          pass_fds=[fd],
+      )
+
+  # listen_sock was automatically closed when leaving the 'with' statement
+  # body. If the daemon process terminated early then the following connect()
+  # will fail with "Connection refused" because no process has the listen
+  # socket open anymore. Launch errors can be detected this way.
+
+  qmp_sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+  qmp_sock.connect(sock_path)
+  ...QMP interaction...
+
+The same socket spawning approach also works with the ``--nbd-server
+addr.type=fd,addr.str=<fd>`` and ``--export
+type=vhost-user-blk,addr.type=fd,addr.str=<fd>`` options.
+
 Export raw image file ``disk.img`` over NBD UNIX domain socket ``nbd.sock``::
 
   $ qemu-storage-daemon \
-- 
2.29.2

Re: [PATCH v3 1/2] docs: show how to spawn qemu-storage-daemon with fd passing

[Daniel P. Berrangé]

On Mon, Mar 01, 2021 at 05:27:27PM +0000, Stefan Hajnoczi wrote:
> The QMP monitor, NBD server, and vhost-user-blk export all support file
> descriptor passing. This is a useful technique because it allows the
> parent process to spawn and wait for qemu-storage-daemon without busy
> waiting, which may delay startup due to arbitrary sleep() calls.
> 
> This Python example is inspired by the test case written for libnbd by
> Richard W.M. Jones <rjones@redhat.com>:
> https://gitlab.com/nbdkit/libnbd/-/commit/89113f484effb0e6c322314ba75c1cbe07a04543
> 
> Thanks to Daniel P. Berrangé <berrange@redhat.com> for suggestions on
> how to get this working. Now let's document it!
> 
> Reported-by: Richard W.M. Jones <rjones@redhat.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> v2:
>  * Use /var/run/qmp.sock instead of /tmp/qmp-$PID.sock to prevent
>    security issues with world-writeable directories [Rich, Daniel]
> ---
>  docs/tools/qemu-storage-daemon.rst | 42 ++++++++++++++++++++++++++++--
>  1 file changed, 40 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v3 2/2] docs: replace insecure /tmp examples in qsd docs

[Stefan Hajnoczi]

World-writeable directories have security issues. Avoid showing them in
the documentation since someone might accidentally use them in
situations where they are insecure.

There tend to be 3 security problems:
1. Denial of service. An adversary may be able to create the file
   beforehand, consume all space/inodes, etc to sabotage us.
2. Impersonation. An adversary may be able to create a listen socket and
   accept incoming connections that were meant for us.
3. Unauthenticated client access. An adversary may be able to connect to
   us if we did not set the uid/gid and permissions correctly.

These can be prevented or mitigated with private /tmp, carefully setting
the umask, etc but that requires special action and does not apply to
all situations. Just avoid using /tmp in examples.

Reported-by: Richard W.M. Jones <rjones@redhat.com>
Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 docs/tools/qemu-storage-daemon.rst | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/docs/tools/qemu-storage-daemon.rst b/docs/tools/qemu-storage-daemon.rst
index 789a8e4a75..2da28a447a 100644
--- a/docs/tools/qemu-storage-daemon.rst
+++ b/docs/tools/qemu-storage-daemon.rst
@@ -69,7 +69,7 @@ Standard options:
   a description of character device properties. A common character device
   definition configures a UNIX domain socket::
 
-  --chardev socket,id=char1,path=/tmp/qmp.sock,server,nowait
+  --chardev socket,id=char1,path=/var/run/qsd-qmp.sock,server,nowait
 
 .. option:: --export [type=]nbd,id=<id>,node-name=<node-name>[,name=<export-name>][,writable=on|off][,bitmap=<name>]
   --export [type=]vhost-user-blk,id=<id>,node-name=<node-name>,addr.type=unix,addr.path=<socket-path>[,writable=on|off][,logical-block-size=<block-size>][,num-queues=<num-queues>]
@@ -108,9 +108,10 @@ Standard options:
   below). TLS encryption can be configured using ``--object`` tls-creds-* and
   authz-* secrets (see below).
 
-  To configure an NBD server on UNIX domain socket path ``/tmp/nbd.sock``::
+  To configure an NBD server on UNIX domain socket path
+  ``/var/run/qsd-nbd.sock``::
 
-  --nbd-server addr.type=unix,addr.path=/tmp/nbd.sock
+  --nbd-server addr.type=unix,addr.path=/var/run/qsd-nbd.sock
 
 .. option:: --object help
   --object <type>,help
-- 
2.29.2

Re: [PATCH v3 2/2] docs: replace insecure /tmp examples in qsd docs

[Daniel P. Berrangé]

On Mon, Mar 01, 2021 at 05:27:28PM +0000, Stefan Hajnoczi wrote:
> World-writeable directories have security issues. Avoid showing them in
> the documentation since someone might accidentally use them in
> situations where they are insecure.
> 
> There tend to be 3 security problems:
> 1. Denial of service. An adversary may be able to create the file
>    beforehand, consume all space/inodes, etc to sabotage us.
> 2. Impersonation. An adversary may be able to create a listen socket and
>    accept incoming connections that were meant for us.
> 3. Unauthenticated client access. An adversary may be able to connect to
>    us if we did not set the uid/gid and permissions correctly.
> 
> These can be prevented or mitigated with private /tmp, carefully setting
> the umask, etc but that requires special action and does not apply to
> all situations. Just avoid using /tmp in examples.
> 
> Reported-by: Richard W.M. Jones <rjones@redhat.com>
> Reported-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  docs/tools/qemu-storage-daemon.rst | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v3 2/2] docs: replace insecure /tmp examples in qsd docs

[Richard W.M. Jones]

For the series:

Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Re: [PATCH v3 0/2] docs: show how to spawn qemu-storage-daemon with fd passing

[Kevin Wolf]

Am 01.03.2021 um 18:27 hat Stefan Hajnoczi geschrieben:
> v3:
>  * Explain how to detect launch errors and that the listen socket must be
>    closed in the parent process in order for this to work [Daniel]
> 
> v2:
>  * Use /var/run/qmp.sock instead of /tmp/qmp-$PID.sock to prevent security
>    issues with world-writeable directories [Rich, Daniel]
>  * Add Patch 2 to fix insecure examples in the documentation [Rich, Daniel]
> 
> Add an example of how to spawn qemu-storage-daemon with fd passing. This
> approach eliminates the need to busy wait for the QMP, NBD, or vhost-user
> socket to become available.

Thanks, applied to the block branch.

Kevin