Re: [Qemu-devel] [PATCH 03/17] rbd: add support for getting password from QCryptoSecret object

["Daniel P. Berrange" <berrange@redhat.com>]

On Mon, Oct 19, 2015 at 03:57:29PM -0700, Josh Durgin wrote:
> On 10/19/2015 08:09 AM, Daniel P. Berrange wrote:
> >Currently RBD passwords must be provided on the command line
> >via
> >
> >   $QEMU -drive file=rbd:pool/image:id=myname:\
> >     key=QVFDVm41aE82SHpGQWhBQXEwTkN2OGp0SmNJY0UrSE9CbE1RMUE=:\
> >     auth_supported=cephx
> >
> >This is insecure because the key is visible in the OS process
> >listing.
> >
> >This adds support for an 'authsecret' parameter in the RBD
> >parameters that can be used with the QCryptoSecret object to
> >provide the password via a file:
> >
> >   echo "QVFDVm41aE82SHpGQWhBQXEwTkN2OGp0SmNJY0UrSE9CbE1RMUE=" > poolkey.b64
> >   $QEMU -object secret,id=secret0,file=poolkey.b64,format=base64 \
> >       -drive file=rbd:pool/image:id=myname:\
> >       auth_supported=cephx,authsecret=secret0
> >
> >Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> >---
> 
> Looks good in general, thanks for fixing this! Just one thing to fix
> below.
> 
> >  block/rbd.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 42 insertions(+)
> >
> >diff --git a/block/rbd.c b/block/rbd.c
> >index a60a19d..0acf777 100644
> >--- a/block/rbd.c
> >+++ b/block/rbd.c
> >@@ -16,6 +16,7 @@
> >  #include "qemu-common.h"
> >  #include "qemu/error-report.h"
> >  #include "block/block_int.h"
> >+#include "crypto/secret.h"
> >
> >  #include <rbd/librbd.h>
> >
> >@@ -228,6 +229,23 @@ static char *qemu_rbd_parse_clientname(const char *conf, char *clientname)
> >      return NULL;
> >  }
> >
> >+
> >+static int qemu_rbd_set_auth(rados_t cluster, const char *secretid,
> >+                             Error **errp)
> >+{
> >+    gchar *secret = qcrypto_secret_lookup_as_base64(secretid,
> >+                                                    errp);
> >+    if (!secret) {
> >+        return -1;
> >+    }
> 
> It looks like this fails if no authsecret is provided. Ceph auth can be
> disabled, so it seems like we should skip the qemu_rbd_set_auth() calls
> in this case.

This failure scenario happens if the user provides a key ID, but the
corresponding QCryptoSecret does not exist. This is a usage error
by the user, so it is appropriate to have it be a fatal error.
In the case that they don't want any auth, they can just leave out
the keyid parameter.

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 :|