Re: [Qemu-block] [PATCH v6 00/16] Implement TLS support to QEMU NBD server & client

[Paolo Bonzini]

On 10/02/2016 19:40, 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
>         v4: 
> https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg04160.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
> 
> The first 2 patches are taken from this other pending patch
> series in order to facilitate merge:
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg00296.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 v6:
> 
>  - Rebase to resolve conflicts with recent qapi changes (Eric)
>    and recent nbd changes (Paolo) which merged
>  - Add MODULE_INIT_QOM to qemu-io & qemu-img to ensure they
>    can load the QIOChannel object types
> 
> Changed in v5:
> 
>  - Pulled in 
> https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg00297.html
>    and applied fixes for issues Eric mentioned in that review
>  - Pulled in 
> https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg00302.html
>  - Rebased to latest git master
> 
> 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 (16):
>   qom: add helpers for UserCreatable object types
>   qemu-nbd: add support for --object command line arg
>   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                           |  54 ++---
>  include/block/nbd.h             |  28 ++-
>  include/monitor/monitor.h       |   3 -
>  include/qom/object_interfaces.h |  92 +++++++++
>  nbd/client.c                    | 440 
> +++++++++++++++++++++++++++++++++++-----
>  nbd/common.c                    |  83 +++++---
>  nbd/nbd-internal.h              |  32 ++-
>  nbd/server.c                    | 334 +++++++++++++++++++++---------
>  qapi/block.json                 |   4 +-
>  qemu-img.c                      |   1 +
>  qemu-io.c                       |   1 +
>  qemu-nbd.c                      | 195 ++++++++++++++----
>  qemu-nbd.texi                   |  13 ++
>  qmp-commands.hx                 |   2 +-
>  qmp.c                           |  76 +------
>  qom/object_interfaces.c         | 174 ++++++++++++++++
>  tests/Makefile                  |   2 +-
>  tests/qemu-iotests/140.out      |   2 +-
>  tests/qemu-iotests/143.out      |   2 +-
>  vl.c                            |  68 +------
>  25 files changed, 1459 insertions(+), 490 deletions(-)
> 

Queued, thanks.

Paolo