From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57918) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aB2pt-0000jd-JF for qemu-devel@nongnu.org; Mon, 21 Dec 2015 10:57:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aB2ps-0004Vw-Ge for qemu-devel@nongnu.org; Mon, 21 Dec 2015 10:57:21 -0500 References: <1450709966-2998-1-git-send-email-berrange@redhat.com> <1450709966-2998-2-git-send-email-berrange@redhat.com> From: Josh Durgin Message-ID: <5678215D.5010406@redhat.com> Date: Mon, 21 Dec 2015 07:57:17 -0800 MIME-Version: 1.0 In-Reply-To: <1450709966-2998-2-git-send-email-berrange@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 1/3] rbd: add support for getting password from QCryptoSecret object List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org Cc: Kevin Wolf , Paolo Bonzini , Jeff Cody , qemu-block@nongnu.org, Ronnie Sahlberg On 12/21/2015 06:59 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 'passwordid' 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 driver=rbd,filename=rbd:pool/image:id=myname:\ > auth_supported=cephx,passwordid=secret0 > > Signed-off-by: Daniel P. Berrange Looks good to me, thanks! Reviewed-by: Josh Durgin > --- > block/rbd.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 47 insertions(+) > > diff --git a/block/rbd.c b/block/rbd.c > index a60a19d..dc4db0a 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,27 @@ static char *qemu_rbd_parse_clientname(const char *conf, char *clientname) > return NULL; > } > > + > +static int qemu_rbd_set_auth(rados_t cluster, const char *passwordid, > + Error **errp) > +{ > + if (passwordid == 0) { > + return 0; > + } > + > + gchar *secret = qcrypto_secret_lookup_as_base64(passwordid, > + errp); > + if (!secret) { > + return -1; > + } > + > + rados_conf_set(cluster, "key", secret); > + g_free(secret); > + > + return 0; > +} > + > + > static int qemu_rbd_set_conf(rados_t cluster, const char *conf, > bool only_read_conf_file, > Error **errp) > @@ -299,10 +321,13 @@ static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp) > char conf[RBD_MAX_CONF_SIZE]; > char clientname_buf[RBD_MAX_CONF_SIZE]; > char *clientname; > + const char *passwordid; > rados_t cluster; > rados_ioctx_t io_ctx; > int ret; > > + passwordid = qemu_opt_get(opts, "passwordid"); > + > if (qemu_rbd_parsename(filename, pool, sizeof(pool), > snap_buf, sizeof(snap_buf), > name, sizeof(name), > @@ -350,6 +375,11 @@ static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp) > return -EIO; > } > > + if (qemu_rbd_set_auth(cluster, passwordid, errp) < 0) { > + rados_shutdown(cluster); > + return -EIO; > + } > + > if (rados_connect(cluster) < 0) { > error_setg(errp, "error connecting"); > rados_shutdown(cluster); > @@ -423,6 +453,11 @@ static QemuOptsList runtime_opts = { > .type = QEMU_OPT_STRING, > .help = "Specification of the rbd image", > }, > + { > + .name = "passwordid", > + .type = QEMU_OPT_STRING, > + .help = "ID of secret providing the password", > + }, > { /* end of list */ } > }, > }; > @@ -436,6 +471,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, > char conf[RBD_MAX_CONF_SIZE]; > char clientname_buf[RBD_MAX_CONF_SIZE]; > char *clientname; > + const char *passwordid; > QemuOpts *opts; > Error *local_err = NULL; > const char *filename; > @@ -450,6 +486,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, > } > > filename = qemu_opt_get(opts, "filename"); > + passwordid = qemu_opt_get(opts, "passwordid"); > > if (qemu_rbd_parsename(filename, pool, sizeof(pool), > snap_buf, sizeof(snap_buf), > @@ -488,6 +525,11 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, > } > } > > + if (qemu_rbd_set_auth(s->cluster, passwordid, errp) < 0) { > + r = -EIO; > + goto failed_shutdown; > + } > + > /* > * Fallback to more conservative semantics if setting cache > * options fails. Ignore errors from setting rbd_cache because the > @@ -919,6 +961,11 @@ static QemuOptsList qemu_rbd_create_opts = { > .type = QEMU_OPT_SIZE, > .help = "RBD object size" > }, > + { > + .name = "passwordid", > + .type = QEMU_OPT_STRING, > + .help = "ID of secret providing the password", > + }, > { /* end of list */ } > } > }; >