[v2,1/3] rbd: add support for getting password from QCryptoSecret object

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 <berrange@redhat.com>
---
 block/rbd.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

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 <berrange@redhat.com>

Looks good to me, thanks!

Reviewed-by: Josh Durgin <jdurgin@redhat.com>

> ---
>   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 <rbd/librbd.h>
>
> @@ -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 */ }
>       }
>   };
>