On Mon, Oct 19, 2015 at 04:09:32PM +0100, Daniel P. Berrange wrote: > There are a variety of places where QEMU needs to have access > to passwords, encryption keys or similar kinds of secrets. > > - VNC / SPICE user passwords > - Curl block http / proxy passwords > - RBD auth password > - iSCSI CHAP password > - x509 private key password > - QCow/QCow2 encryption key > > QEMU has a variety of ways of dealing with this problem, some > good, some ugly, some bad. > > - The RBD block driver accepts the password in plaintext > via a private RBD config option. This is a pending CVE > > https://security-tracker.debian.org/tracker/CVE-2015-5160 > > - The iSCSI driver accepts the password in plaintext as > a block driver option. This is the same as the RBD CVE > essentially, just a QEMU option, rather than a librbd > option > > - The VNC / SPICE servers only accept the passwords via > the QEMU monitor. This is secure, but it inconvenient > for adhoc developer usage where security of CLI args > does not matter. > > - QCow/QCow2 encryption keys can be provided by the monitor > but this is not available for qemu-img, qemu-io and > qemu-nbd. QEMU falls back to doing interactive > console prompting to get keys. > > - x509 privte key passwords are not supported at all by > QEMU which forces users to store their key in plaintext > on their host FS. > > - The CURL driver doesn't support HTTP auth at all > currently. > > 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. > > 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. > > 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
Acked-by: Stefan Hajnoczi <stefa...@redhat.com> i.e. makes sense to me but I haven't reviewed patches in detail