Re: [Qemu-devel] [PATCH v3 0/3] Use QCryptoSecret for block device passwords

[Daniel P. Berrange]

On Tue, Jan 19, 2016 at 09:41:31PM +0100, Paolo Bonzini wrote:
> 
> 
> On 19/01/2016 17:46, Daniel P. Berrange wrote:
> > On Tue, Jan 19, 2016 at 05:32:35PM +0100, Paolo Bonzini wrote:
> >>
> >>
> >> On 19/01/2016 14:51, Daniel P. Berrange wrote:
> >>> This series was previously posted:
> >>>
> >>>   v1: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html
> >>>   v2: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg03809.html
> >>>
> >>> The RBD, Curl and iSCSI block device drivers all need the ability
> >>> to accept a password to authenticate with the remote network storage
> >>> server. Currently RBD and iSCSI both just take the password in clear
> >>> text as part of the block parameters which is insecure (passwords are
> >>> visible in the process listing), while Curl doesn't support auth at
> >>> all.
> >>>
> >>> This series updates all three drivers so that they use the recently
> >>> merged QCryptoSecret API for getting passwords. Each driver gains
> >>> a 'passwordid' property that can be set to provide the ID of a
> >>> QCryptoSecret object instance, which in turn provides the actual
> >>> password data.
> >>>
> >>> This series is required in order to fix a long standing CVE security
> >>> flaw in libvirt, whereby passwords are exposed in the command line
> >>> arguments and so visible in process listing
> >>>
> >>> This series would benefit from the --object additions to qemu-img,
> >>> qemu-io and qemu-nbd, but this is not a pre-requisite for its merge
> >>> as it us still useful in the system emulator without that support:
> >>>
> >>>   https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03381.html
> >>>
> >>> Changed in v3:
> >>>
> >>>  - Rename 'passwordid' to 'password-id', 'proxypasswordid'
> >>>    to 'proxy-password-id' and 'proxyusername' to 'proxy-username'
> >>>    (Markus)
> >>>
> >>> Daniel P. Berrange (3):
> >>>   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
> >>>
> >>>  block/curl.c  | 66 
> >>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>  block/iscsi.c | 24 +++++++++++++++++++++-
> >>>  block/rbd.c   | 47 ++++++++++++++++++++++++++++++++++++++++++
> >>>  3 files changed, 136 insertions(+), 1 deletion(-)
> >>>
> >>
> >> Apologizing in advance for bikeshedding: what about using proxy-secret
> >> and secret instead?  Traditionally the name of object options has
> >> referred to the name of the class.
> > 
> > I wanted to avoid using the word 'secret', because in the future when we
> > have ability to run LUKS encryption over any backend, we will have need
> > to pass multiple secrets for a single drive spec. For example, we'll need
> > one secret to provide the RBD password, and one secret to provide the LUKS
> > decryption passphrase. So I felt using 'password' is a better choice to
> > standardize on for the protocol authentication needs.
> 
> If you have a qcow2->luks->rbd tree, the LUKS passphrase would be
> file.secret, while the rbd credentials would be file.file.username and
> file.file.secret.

Yep, I just feel that could be slightly confusing as to which is
which if they have the same name.

> password-secret and proxy-password-secret are fine too.  I just don't
> like "id" too much.

That sounds ok to me.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|