Re: [Qemu-devel] [PATCH 00/17] Framework for securely passing secrets to QEMU

[Dr. David Alan Gilbert]

* Daniel P. Berrange (address@hidden) wrote:

<snip>

> It is obvious there there is a wide variety of functionality
> in QEMU that needs access to "secrets". This need will only
> grow over time. We need to stop having everyone invent their
> own dangerous wheels and provide a standard mechanism for
> securely passing secrets to QEMU.

Agreed.

> 
> To this end, this series introduces a QCryptoSecret object
> class with short name "secret". All the places which needs
> passwords/keys are then converted to get their via this
> API, except VNC/SPICE which are a future exercise.
> 
> Example usage for creating secrets...
> 
> Direct password, insecure, for ad-hoc developer testing only
> 
>   $QEMU -object secret,id=sec0,data=letmein
> 
> Indirect password via a file, good for production
> 
>   echo -n "letmein" > mypasswd.txt
>   $QEMU -object secret,id=sec0,file=mypasswd.txt
> 
> The file based approach supports file descriptor passing,
> so mgmt apps can use that to dynamically add passwords to
> running QEMU.

Would it make any sense to support the Linux kernel key system?
That seems to have a way of passing keys between a limited
set of processes and protecting them using SELinux and the like.

(I can also imagine that people might want to feed it with keys
from a TPM or other hardware security device, but fortunately
I can't remember enough about TPMs to remember how getting
keys worked).

Dave

> 
> There is a better way though, which is to use an encrypted
> secret. The intent here is that a mgmt app (like libvirt)
> will generate a random AES-256 key for each virtual machine
> it starts (saved in eg /var/run/libvirt/qemu/$GUEST.key)
> It can then use the direct password passing, but encrypt
> the data.
> 
>   $QEMU \
>     -object 
> secret,id=secmaster0,file=/var/run/libvirt/qemu/$GUEST.key,format=base64 \
>     -object secret,id=sec0,data=[base64 ciphertext],\
>                keyid=secmaster0,iv=[base64 initialization vector]
> 
> This means that the mgmt app does not need to worry about
> file descriptor passing at all. It can just use regular
> object properties, safe in the knowledge that the data is
> protected by a secret AES key shared only between QEMU
> and the mgmt app.
> 
> Use of encrypted secrets is not restricted to directly
> provided inline data. If the secret is stored in an
> external file, that can be encrypted too
> 
>   $QEMU \
>     -object 
> secret,id=secmaster0,file=/var/run/libvirt/qemu/$GUEST.key,format=base64 \
>     -object secret,id=sec0,file=/some/secret/file.aes,\
>                keyid=secmaster0,iv=[base64 initialization vector]
> 
> 
> 
> Example usage for referencing secrets...
> 
> CURL:
> 
>   $QEMU -object secret,id=sec0... \
>      -drive driver=http,url=http://example.com/someimg.qcow2,\
>               user=dan,passwordid=sec0
> 
>   $QEMU -object secret,id=sec0... -object secret,id=sec1 \
>      -drive driver=http,url=http://example.com/someimg.qcow2,\
>               user=dan,passwordid=sec0,proxyuser=dan,passwordid=sec1
> 
> iSCSI:
> 
>   $QEMU -object secret,id=sec0... \
>      -drive driver=iscsi,url=iscsi://example.com/target-foo/lun1,\
>              user=dan,passwordid=sec0
> 
> RBD:
> 
>   $QEMU -object secret,id=sec0... \
>      -drive driver=rbd,file=rbd:pool/image:id=myname,\
>              auth-supported-cephx,authsecret=sec0
> 
> QCow/Qcow2 encryption
> 
>   $QEMU -object secret,id=sec0... \
>      -drive file=someimage.qcow2,keyid=sec0
> 
> 
> Finally, this extends qemu-img, qemu-nbd and qemu-io. All of
> them gain a new '--object' parameter which provides the same
> functionality as '-object' with QEMU system emulators. This
> lets us create QCryptoSecret object instances in those tools
> 
> The tools also then get support for a new '--source IMG-OPTS'
> parameter to allow a full set of image options to be specified,
> instead of just separate hardcoded args for format + filename
> which they currently permit. This is probably the area I am
> least sure of. I struggled to understand what the "best
> practice" is for turning a QemuOpts into something you can
> use to instantiate block backends. So I may well have not
> done the right thing.
> 
> Towards the end I rip out the current encryption key handling
> from the block layer so all the hairy code for dealing
> with encrypted block devices disappears, and encryption
> can be a 100% private matter for the block driver internal
> impl.  This is obviously not backwards compatible, but we
> have been warning users we're dropping qcow2 encryption
> support for a while.
> 
> Finally I disable support for writing to encrypted qcow2
> files, but keep the ability to read them, so users can
> liberate data. Originally we were intending to fully
> delete encryption support, due to the burden it places
> on the internal boock API. Since I removed that burden
> I figured it is reasonable to keep read-only support
> around.
> 
> The only real missing thing is wiring up the VNC/SPICE
> servers. There is one complication here in that it is
> common to change the VNC/SPICE password at runtime, and
> I'm not sure what the best way to deal with this is.
> 
> There are two obvious choices
> 
>  a. Create a new secret, tell the VNC server to use
>     the new secret, delete the old secret. This will
>     need a new 'graphics_secret_update' command in
>     the monitor, to use alongside object_add/del.
> 
>  b. Allow the existing secret to be updated via some
>     new 'object_update' method, and internally notify
>     the VNC/SPICE server when the secret is updated.
>     This would probably need a new QOM interface
>     UserUpdatableObject to be defined, as an refinement
>     of UserCreatableObject.
> 
> Daniel P. Berrange (17):
>   crypto: add QCryptoSecret object class for password/key handling
>   crypto: add support for loading encrypted x509 keys
>   rbd: add support for getting password from QCryptoSecret object
>   curl: add support for HTTP authentication parameters
>   iscsi: add support for getting CHAP password via QCryptoSecret API
>   qcow: add a 'keyid' parameter to qcow options
>   qcow2: add a 'keyid' parameter to qcow2 options
>   qom: add user_creatable_add & user_creatable_del methods
>   qemu-img: add support for --object command line arg
>   qemu-nbd: add support for --object command line arg
>   qemu-io: add support for --object command line arg
>   qemu-io: allow specifying image as a set of options args
>   qemu-nbd: allow specifying image as a set of options args
>   qemu-img: allow specifying image as a set of options args
>   block: rip out all traces of password prompting
>   block: remove all encryption handling APIs
>   block: remove support for writing to qcow/qcow2 encrypted images
> 
>  block.c                         |  88 +----
>  block/curl.c                    |  66 ++++
>  block/iscsi.c                   |  24 +-
>  block/qapi.c                    |   2 +-
>  block/qcow.c                    | 122 +++++--
>  block/qcow2.c                   | 116 +++---
>  block/qcow2.h                   |   1 +
>  block/rbd.c                     |  42 +++
>  blockdev.c                      |  69 +---
>  crypto/Makefile.objs            |   1 +
>  crypto/secret.c                 | 513 ++++++++++++++++++++++++++
>  crypto/tlscredsx509.c           |  47 +++
>  hmp.c                           |  42 +--
>  hw/usb/dev-storage.c            |  32 --
>  include/block/block.h           |   5 +-
>  include/crypto/secret.h         | 139 +++++++
>  include/crypto/tlscredsx509.h   |   1 +
>  include/monitor/monitor.h       |  10 -
>  include/qemu/option.h           |   1 +
>  include/qemu/osdep.h            |   2 -
>  include/qom/object_interfaces.h |  31 ++
>  monitor.c                       |  65 ----
>  qapi/block-core.json            |  23 +-
>  qapi/crypto.json                |  14 +
>  qemu-img-cmds.hx                |  44 +--
>  qemu-img.c                      | 788 
> +++++++++++++++++++++++++++++++++++-----
>  qemu-img.texi                   |   8 +
>  qemu-io.c                       | 145 ++++++--
>  qemu-nbd.c                      | 142 +++++++-
>  qemu-nbd.texi                   |   7 +
>  qemu-options.hx                 |  84 ++++-
>  qmp.c                           |  83 +----
>  qom/object_interfaces.c         |  76 ++++
>  tests/.gitignore                |   1 +
>  tests/Makefile                  |   2 +
>  tests/qemu-iotests/087          |  20 +
>  tests/qemu-iotests/087.out      |  26 +-
>  tests/qemu-iotests/134          |  17 +-
>  tests/qemu-iotests/134.out      |  44 +--
>  tests/qemu-iotests/common.rc    |   4 +-
>  tests/test-crypto-secret.c      | 440 ++++++++++++++++++++++
>  util/oslib-posix.c              |  66 ----
>  util/oslib-win32.c              |  24 --
>  util/qemu-option.c              |   6 +
>  vl.c                            |   8 +-
>  45 files changed, 2740 insertions(+), 751 deletions(-)
>  create mode 100644 crypto/secret.c
>  create mode 100644 include/crypto/secret.h
>  create mode 100644 tests/test-crypto-secret.c
> 
> -- 
> 2.4.3
> 
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK