Re: [Qemu-devel] [PATCH v4 00/14] Implement TLS support to QEMU NBD server & client

[Paolo Bonzini <pbonzini@redhat.com>]

On 21/01/2016 17:37, Daniel P. Berrange wrote:
> This is an update of the series previously posted:
> 
>   v1: https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg06126.html
>   v2: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg01580.html
>   v3: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03440.html
> 
> This series of patches implements support for TLS in the QEMU NBD
> server and client code.
> 
> It is implementing the NBD_OPT_STARTTLS option that was previously
> discussed here:
> 
>   https://www.redhat.com/archives/libvir-list/2014-October/msg00506.html
> 
> And is also described in the NBD spec here:
> 
>   https://github.com/yoe/nbd/blob/master/doc/proto.md
> 
> To ensure that clients always get a suitable error message from the
> NBD server when it is configured with TLS, a client speaking the
> new style protocol will always send NBD_OPT_LIST as the first thing
> it does, so that we can see the NBD_REP_ERR_TLS_REQD response. This
> should all be backwards & forwards compatible with previous QEMU
> impls of NBD
> 
> Usage of TLS is described in the commit messages for each patch,
> but for sake of people who don't want to explore the series, here's
> the summary
> 
> Starting QEMU system emulator with a disk backed by an TLS encrypted
> NBD export
> 
>   $ qemu-system-x86_64 \
>      -object tls-creds-x509,id=tls0,endpoint=client,dir=/home/berrange/security/qemutls \
>      -drive driver=nbd,host=localhost,port=9000,tls-creds=tls0
> 
> Starting a standalone NBD server providing a TLS encrypted NBD export
> 
>   $ qemu-nbd \
>      --object tls-creds-x509,id=tls0,endpoint=server,dir=/home/berrange/security/qemutls
>      --tls-creds tls0 \
>      --export-name default \
>      $IMAGEFILE
> 
> The --export-name is optional, if omitted, the default "" will
> be used.
> 
> Starting a QEMU system emulator built-in NBD server
> 
>   $ qemu-system-x86_64 \
>      -qmp unix:/tmp/qmp,server \
>      -hda /home/berrange/Fedora-Server-netinst-x86_64-23.iso \
>      -object tls-creds-x509,id=tls0,dir=/home/berrange/security/qemutls,endpoint=server
> 
>   $ qmp-shell /tmp/qmp
>     (qmp) nbd-server-start addr={"host":"localhost","port":"9000"} tls-creds=tls0
>     (qmp) nbd-server-add device=ide0-hd0
> 
> This series depends on this bug fix I recently sent:
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03406.html
> 
> And the qemu-nbd/etc command line options work
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03381.html
> 
> The first 4 patches are the conversion to the I/O channels
> framework.
> 
> The next 6 patches are general tweaks to QEMU's impl of the
> NBD protocol for better compliance and/or future proofing.
> 
> The next patch provides the NBD protocol TLS implementation.
> 
> The final 3 patches allow TLS to be enabled in the QEMU NBD
> client and servers.
> 
> Changed in v4:
> 
>  - Don't pick the first export name in the list if no export
>    name is provided (Paolo)
>  - Set client requested export name to "" if none is provided
>    by the user (Paolo)
>  - Set server advertized export name to "" if TLS is enabled
>    and none is provided by the user (Paolo)
>  - Rename qemu-nbd --exportname to --export-name (Paolo)
>  - Use iov_discard_front() to simplify iov handling (Paolo)
> 
> Changed in v3:
> 
>  - Rebase to resolve conflicts with recently merged NBD patches
> 
> Changed in v2:
> 
>  - Fix error codes used during NBD TLS option negotiate
>  - Update patch with helpers for UserCreatable object types
> 
> Daniel P. Berrange (14):
>   nbd: convert block client to use I/O channels for connection setup
>   nbd: convert qemu-nbd server to use I/O channels for connection setup
>   nbd: convert blockdev NBD server to use I/O channels for connection
>     setup
>   nbd: convert to using I/O channels for actual socket I/O
>   nbd: invert client logic for negotiating protocol version
>   nbd: make server compliant with fixed newstyle spec
>   nbd: make client request fixed new style if advertized
>   nbd: allow setting of an export name for qemu-nbd server
>   nbd: always query export list in fixed new style protocol
>   nbd: use "" as a default export name if none provided
>   nbd: implement TLS support in the protocol negotiation
>   nbd: enable use of TLS with NBD block driver
>   nbd: enable use of TLS with qemu-nbd server
>   nbd: enable use of TLS with nbd-server-start command
> 
>  Makefile            |   6 +-
>  block/nbd-client.c  |  91 +++++++----
>  block/nbd-client.h  |  10 +-
>  block/nbd.c         | 105 ++++++++++---
>  blockdev-nbd.c      | 131 +++++++++++++---
>  hmp.c               |   2 +-
>  include/block/nbd.h |  28 +++-
>  nbd/client.c        | 440 +++++++++++++++++++++++++++++++++++++++++++++-------
>  nbd/common.c        |  83 ++++++----
>  nbd/nbd-internal.h  |  32 ++--
>  nbd/server.c        | 334 ++++++++++++++++++++++++++++-----------
>  qapi/block.json     |   4 +-
>  qemu-nbd.c          | 159 ++++++++++++++-----
>  qemu-nbd.texi       |   7 +
>  qmp-commands.hx     |   2 +-
>  tests/Makefile      |   2 +-
>  16 files changed, 1123 insertions(+), 313 deletions(-)
> 

Looks good, but I cannot apply it without the command line options...

Paolo