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 <berra...@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 :|