From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54832) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZoSO4-0005jd-9B for qemu-devel@nongnu.org; Tue, 20 Oct 2015 04:35:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZoSO0-0006I1-5U for qemu-devel@nongnu.org; Tue, 20 Oct 2015 04:35:16 -0400 Date: Tue, 20 Oct 2015 09:35:00 +0100 From: "Daniel P. Berrange" Message-ID: <20151020083500.GB14616@redhat.com> References: <1445267389-21846-1-git-send-email-berrange@redhat.com> <1445267389-21846-4-git-send-email-berrange@redhat.com> <56257559.1060001@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <56257559.1060001@redhat.com> Subject: Re: [Qemu-devel] [PATCH 03/17] rbd: add support for getting password from QCryptoSecret object Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Josh Durgin Cc: Kevin Wolf , Ronnie Sahlberg , qemu-block@nongnu.org, qemu-devel@nongnu.org, Markus Armbruster , Stefan Hajnoczi , Paolo Bonzini 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 > >--- > > 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 > > > >@@ -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 :|