qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: "Daniel P. Berrange" <berrange@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Josh Durgin <jdurgin@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	qemu-block@nongnu.org, qemu-devel@nongnu.org,
	Markus Armbruster <armbru@redhat.com>,
	Ronnie Sahlberg <ronniesahlberg@gmail.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 00/17] Framework for securely passing secrets to QEMU
Date: Mon, 19 Oct 2015 18:13:24 +0100	[thread overview]
Message-ID: <20151019171324.GF2462@work-vm> (raw)
In-Reply-To: <1445267389-21846-1-git-send-email-berrange@redhat.com>

* Daniel P. Berrange (berrange@redhat.com) 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 / dgilbert@redhat.com / Manchester, UK

  parent reply	other threads:[~2015-10-19 17:13 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-19 15:09 [Qemu-devel] [PATCH 00/17] Framework for securely passing secrets to QEMU Daniel P. Berrange
2015-10-19 15:09 ` [Qemu-devel] [PATCH 01/17] crypto: add QCryptoSecret object class for password/key handling Daniel P. Berrange
2015-10-19 15:18   ` Paolo Bonzini
2015-10-19 15:24     ` Daniel P. Berrange
2015-10-19 15:40       ` Paolo Bonzini
2015-10-19 15:46         ` Daniel P. Berrange
2015-10-19 16:12           ` Paolo Bonzini
2015-10-19 16:24             ` Daniel P. Berrange
2015-10-19 16:28               ` Paolo Bonzini
2015-10-19 16:30                 ` Daniel P. Berrange
2015-10-19 15:09 ` [Qemu-devel] [PATCH 02/17] crypto: add support for loading encrypted x509 keys Daniel P. Berrange
2015-10-19 15:09 ` [Qemu-devel] [PATCH 03/17] rbd: add support for getting password from QCryptoSecret object Daniel P. Berrange
2015-10-19 22:57   ` Josh Durgin
2015-10-20  8:35     ` Daniel P. Berrange
2015-10-19 15:09 ` [Qemu-devel] [PATCH 04/17] curl: add support for HTTP authentication parameters Daniel P. Berrange
2015-10-19 15:09 ` [Qemu-devel] [PATCH 05/17] iscsi: add support for getting CHAP password via QCryptoSecret API Daniel P. Berrange
2015-10-19 15:09 ` [Qemu-devel] [PATCH 06/17] qcow: add a 'keyid' parameter to qcow options Daniel P. Berrange
2015-10-28 13:56   ` Eric Blake
2015-10-19 15:09 ` [Qemu-devel] [PATCH 07/17] qcow2: add a 'keyid' parameter to qcow2 options Daniel P. Berrange
2015-10-19 23:29   ` Eric Blake
2015-10-28 13:58     ` Eric Blake
2015-10-19 15:09 ` [Qemu-devel] [PATCH 08/17] qom: add user_creatable_add & user_creatable_del methods Daniel P. Berrange
2015-10-19 15:09 ` [Qemu-devel] [PATCH 09/17] qemu-img: add support for --object command line arg Daniel P. Berrange
2015-10-19 15:09 ` [Qemu-devel] [PATCH 10/17] qemu-nbd: " Daniel P. Berrange
2015-10-19 15:09 ` [Qemu-devel] [PATCH 11/17] qemu-io: " Daniel P. Berrange
2015-10-19 15:09 ` [Qemu-devel] [PATCH 12/17] qemu-io: allow specifying image as a set of options args Daniel P. Berrange
2015-10-19 15:09 ` [Qemu-devel] [PATCH 13/17] qemu-nbd: " Daniel P. Berrange
2015-10-19 15:09 ` [Qemu-devel] [PATCH 14/17] qemu-img: " Daniel P. Berrange
2015-10-19 15:09 ` [Qemu-devel] [PATCH 15/17] block: rip out all traces of password prompting Daniel P. Berrange
2015-10-19 15:09 ` [Qemu-devel] [PATCH 16/17] block: remove all encryption handling APIs Daniel P. Berrange
2015-10-19 15:09 ` [Qemu-devel] [PATCH 17/17] block: remove support for writing to qcow/qcow2 encrypted images Daniel P. Berrange
2015-10-19 16:05 ` [Qemu-devel] [PATCH 00/17] Framework for securely passing secrets to QEMU Alex Bennée
2015-10-19 16:14   ` Daniel P. Berrange
2015-10-19 17:13 ` Dr. David Alan Gilbert [this message]
2015-10-19 17:46   ` Daniel P. Berrange
2015-10-23 15:31 ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151019171324.GF2462@work-vm \
    --to=dgilbert@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=jdurgin@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=ronniesahlberg@gmail.com \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).