[Qemu-devel] [PATCH v3 0/3] Use QCryptoSecret for block device passwords

[Qemu-devel] [PATCH v3 0/3] Use QCryptoSecret for block device passwords

[Daniel P. Berrange]

This series was previously posted:

  v1: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html
  v2: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg03809.html

The RBD, Curl and iSCSI block device drivers all need the ability
to accept a password to authenticate with the remote network storage
server. Currently RBD and iSCSI both just take the password in clear
text as part of the block parameters which is insecure (passwords are
visible in the process listing), while Curl doesn't support auth at
all.

This series updates all three drivers so that they use the recently
merged QCryptoSecret API for getting passwords. Each driver gains
a 'passwordid' property that can be set to provide the ID of a
QCryptoSecret object instance, which in turn provides the actual
password data.

This series is required in order to fix a long standing CVE security
flaw in libvirt, whereby passwords are exposed in the command line
arguments and so visible in process listing

This series would benefit from the --object additions to qemu-img,
qemu-io and qemu-nbd, but this is not a pre-requisite for its merge
as it us still useful in the system emulator without that support:

  https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03381.html

Changed in v3:

 - Rename 'passwordid' to 'password-id', 'proxypasswordid'
   to 'proxy-password-id' and 'proxyusername' to 'proxy-username'
   (Markus)

Daniel P. Berrange (3):
  rbd: add support for getting password from QCryptoSecret object
  curl: add support for HTTP authentication parameters
  iscsi: add support for getting CHAP password via QCryptoSecret API

 block/curl.c  | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 block/iscsi.c | 24 +++++++++++++++++++++-
 block/rbd.c   | 47 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 136 insertions(+), 1 deletion(-)

-- 
2.5.0

[Qemu-devel] [PATCH v3 1/3] rbd: add support for getting password from QCryptoSecret object

[Daniel P. Berrange]

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 'password-id' 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,password-id=secret0

Reviewed-by: Josh Durgin <jdurgin@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 block/rbd.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/block/rbd.c b/block/rbd.c
index a60a19d..de49f1e 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, "password-id");
+
     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 = "password-id",
+            .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, "password-id");
 
     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 = "password-id",
+            .type = QEMU_OPT_STRING,
+            .help = "ID of secret providing the password",
+        },
         { /* end of list */ }
     }
 };
-- 
2.5.0

[Qemu-devel] [PATCH v3 2/3] curl: add support for HTTP authentication parameters

[Daniel P. Berrange]

If connecting to a web server which has authentication
turned on, QEMU gets a 401 as curl has not been configured
with any authentication credentials.

This adds 4 new parameters to the curl block driver
options, username, password-id, proxy-username and
proxy-password-id. Passwords are provided using the
recently added 'secret' object type

 $QEMU \
     -object secret,id=sec0,filename=/home/berrange/example.pw \
     -object secret,id=sec1,filename=/home/berrange/proxy.pw \
     -drive driver=http,url=http://example.com/some.img,\
            username=dan,password-id=sec0,\
            proxy-username=dan,proxy-password-id=sec1

Of course it is possible to use the same secret for both the
proxy & server passwords if desired, or omit the proxy auth
details, or the server auth details as required.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 block/curl.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/block/curl.c b/block/curl.c
index 8994182..b7e412e 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -26,6 +26,7 @@
 #include "block/block_int.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qstring.h"
+#include "crypto/secret.h"
 #include <curl/curl.h>
 
 // #define DEBUG_CURL
@@ -77,6 +78,10 @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle,
 #define CURL_BLOCK_OPT_SSLVERIFY "sslverify"
 #define CURL_BLOCK_OPT_TIMEOUT "timeout"
 #define CURL_BLOCK_OPT_COOKIE    "cookie"
+#define CURL_BLOCK_OPT_USERNAME "username"
+#define CURL_BLOCK_OPT_PASSWORDID "password-id"
+#define CURL_BLOCK_OPT_PROXY_USERNAME "proxy-username"
+#define CURL_BLOCK_OPT_PROXY_PASSWORDID "proxy-password-id"
 
 struct BDRVCURLState;
 
@@ -119,6 +124,10 @@ typedef struct BDRVCURLState {
     char *cookie;
     bool accept_range;
     AioContext *aio_context;
+    char *username;
+    char *password;
+    char *proxyusername;
+    char *proxypassword;
 } BDRVCURLState;
 
 static void curl_clean_state(CURLState *s);
@@ -418,6 +427,21 @@ static CURLState *curl_init_state(BlockDriverState *bs, BDRVCURLState *s)
         curl_easy_setopt(state->curl, CURLOPT_ERRORBUFFER, state->errmsg);
         curl_easy_setopt(state->curl, CURLOPT_FAILONERROR, 1);
 
+        if (s->username) {
+            curl_easy_setopt(state->curl, CURLOPT_USERNAME, s->username);
+        }
+        if (s->password) {
+            curl_easy_setopt(state->curl, CURLOPT_PASSWORD, s->password);
+        }
+        if (s->proxyusername) {
+            curl_easy_setopt(state->curl,
+                             CURLOPT_PROXYUSERNAME, s->proxyusername);
+        }
+        if (s->proxypassword) {
+            curl_easy_setopt(state->curl,
+                             CURLOPT_PROXYPASSWORD, s->proxypassword);
+        }
+
         /* Restrict supported protocols to avoid security issues in the more
          * obscure protocols.  For example, do not allow POP3/SMTP/IMAP see
          * CVE-2013-0249.
@@ -524,10 +548,31 @@ static QemuOptsList runtime_opts = {
             .type = QEMU_OPT_STRING,
             .help = "Pass the cookie or list of cookies with each request"
         },
+        {
+            .name = CURL_BLOCK_OPT_USERNAME,
+            .type = QEMU_OPT_STRING,
+            .help = "Username for HTTP auth"
+        },
+        {
+            .name = CURL_BLOCK_OPT_PASSWORDID,
+            .type = QEMU_OPT_STRING,
+            .help = "ID of secret used as password for HTTP auth",
+        },
+        {
+            .name = CURL_BLOCK_OPT_PROXY_USERNAME,
+            .type = QEMU_OPT_STRING,
+            .help = "Username for HTTP proxy auth"
+        },
+        {
+            .name = CURL_BLOCK_OPT_PROXY_PASSWORDID,
+            .type = QEMU_OPT_STRING,
+            .help = "ID of secret used as password for HTTP proxy auth",
+        },
         { /* end of list */ }
     },
 };
 
+
 static int curl_open(BlockDriverState *bs, QDict *options, int flags,
                      Error **errp)
 {
@@ -538,6 +583,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
     const char *file;
     const char *cookie;
     double d;
+    const char *passwordid;
 
     static int inited = 0;
 
@@ -579,6 +625,26 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
         goto out_noclean;
     }
 
+    s->username = g_strdup(qemu_opt_get(opts, CURL_BLOCK_OPT_USERNAME));
+    passwordid = qemu_opt_get(opts, CURL_BLOCK_OPT_PASSWORDID);
+
+    if (passwordid) {
+        s->password = qcrypto_secret_lookup_as_utf8(passwordid, errp);
+        if (!s->password) {
+            goto out_noclean;
+        }
+    }
+
+    s->proxyusername = g_strdup(
+        qemu_opt_get(opts, CURL_BLOCK_OPT_PROXY_USERNAME));
+    passwordid = qemu_opt_get(opts, CURL_BLOCK_OPT_PROXY_PASSWORDID);
+    if (passwordid) {
+        s->proxypassword = qcrypto_secret_lookup_as_utf8(passwordid, errp);
+        if (!s->proxypassword) {
+            goto out_noclean;
+        }
+    }
+
     if (!inited) {
         curl_global_init(CURL_GLOBAL_ALL);
         inited = 1;
-- 
2.5.0

[Qemu-devel] [PATCH v3 3/3] iscsi: add support for getting CHAP password via QCryptoSecret API

[Daniel P. Berrange]

The iSCSI driver currently accepts the CHAP password in plain text
as a block driver property. This change adds a new "password-id"
property that accepts the ID of a QCryptoSecret instance.

  $QEMU \
     -object secret,id=sec0,filename=/home/berrange/example.pw \
     -drive driver=iscsi,url=iscsi://example.com/target-foo/lun1,\
            user=dan,password-id=sec0

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 block/iscsi.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 3acb052..6a84ae8 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -39,6 +39,7 @@
 #include "sysemu/sysemu.h"
 #include "qmp-commands.h"
 #include "qapi/qmp/qstring.h"
+#include "crypto/secret.h"
 
 #include <iscsi/iscsi.h>
 #include <iscsi/scsi-lowlevel.h>
@@ -1075,6 +1076,8 @@ static void parse_chap(struct iscsi_context *iscsi, const char *target,
     QemuOpts *opts;
     const char *user = NULL;
     const char *password = NULL;
+    const char *passwordid;
+    char *secret = NULL;
 
     list = qemu_find_opts("iscsi");
     if (!list) {
@@ -1094,8 +1097,20 @@ static void parse_chap(struct iscsi_context *iscsi, const char *target,
         return;
     }
 
+    passwordid = qemu_opt_get(opts, "password-id");
     password = qemu_opt_get(opts, "password");
-    if (!password) {
+    if (passwordid && password) {
+        error_setg(errp, "'password' and 'password-id' properties are "
+                   "mutually exclusive");
+        return;
+    }
+    if (passwordid) {
+        secret = qcrypto_secret_lookup_as_utf8(passwordid, errp);
+        if (!secret) {
+            return;
+        }
+        password = secret;
+    } else if (!password) {
         error_setg(errp, "CHAP username specified but no password was given");
         return;
     }
@@ -1103,6 +1118,8 @@ static void parse_chap(struct iscsi_context *iscsi, const char *target,
     if (iscsi_set_initiator_username_pwd(iscsi, user, password)) {
         error_setg(errp, "Failed to set initiator username and password");
     }
+
+    g_free(secret);
 }
 
 static void parse_header_digest(struct iscsi_context *iscsi, const char *target,
@@ -1853,6 +1870,11 @@ static QemuOptsList qemu_iscsi_opts = {
             .type = QEMU_OPT_STRING,
             .help = "password for CHAP authentication to target",
         },{
+            .name = "password-id",
+            .type = QEMU_OPT_STRING,
+            .help = "ID of the secret providing password for CHAP "
+                    "authentication to target",
+        },{
             .name = "header-digest",
             .type = QEMU_OPT_STRING,
             .help = "HeaderDigest setting. "
-- 
2.5.0

Re: [Qemu-devel] [PATCH v3 0/3] Use QCryptoSecret for block device passwords

[Paolo Bonzini]

On 19/01/2016 14:51, Daniel P. Berrange wrote:
> This series was previously posted:
> 
>   v1: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html
>   v2: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg03809.html
> 
> The RBD, Curl and iSCSI block device drivers all need the ability
> to accept a password to authenticate with the remote network storage
> server. Currently RBD and iSCSI both just take the password in clear
> text as part of the block parameters which is insecure (passwords are
> visible in the process listing), while Curl doesn't support auth at
> all.
> 
> This series updates all three drivers so that they use the recently
> merged QCryptoSecret API for getting passwords. Each driver gains
> a 'passwordid' property that can be set to provide the ID of a
> QCryptoSecret object instance, which in turn provides the actual
> password data.
> 
> This series is required in order to fix a long standing CVE security
> flaw in libvirt, whereby passwords are exposed in the command line
> arguments and so visible in process listing
> 
> This series would benefit from the --object additions to qemu-img,
> qemu-io and qemu-nbd, but this is not a pre-requisite for its merge
> as it us still useful in the system emulator without that support:
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03381.html
> 
> Changed in v3:
> 
>  - Rename 'passwordid' to 'password-id', 'proxypasswordid'
>    to 'proxy-password-id' and 'proxyusername' to 'proxy-username'
>    (Markus)
> 
> Daniel P. Berrange (3):
>   rbd: add support for getting password from QCryptoSecret object
>   curl: add support for HTTP authentication parameters
>   iscsi: add support for getting CHAP password via QCryptoSecret API
> 
>  block/curl.c  | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  block/iscsi.c | 24 +++++++++++++++++++++-
>  block/rbd.c   | 47 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 136 insertions(+), 1 deletion(-)
> 

Apologizing in advance for bikeshedding: what about using proxy-secret
and secret instead?  Traditionally the name of object options has
referred to the name of the class.

Paolo

Re: [Qemu-devel] [PATCH v3 0/3] Use QCryptoSecret for block device passwords

[Daniel P. Berrange]

On Tue, Jan 19, 2016 at 05:32:35PM +0100, Paolo Bonzini wrote:
> 
> 
> On 19/01/2016 14:51, Daniel P. Berrange wrote:
> > This series was previously posted:
> > 
> >   v1: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html
> >   v2: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg03809.html
> > 
> > The RBD, Curl and iSCSI block device drivers all need the ability
> > to accept a password to authenticate with the remote network storage
> > server. Currently RBD and iSCSI both just take the password in clear
> > text as part of the block parameters which is insecure (passwords are
> > visible in the process listing), while Curl doesn't support auth at
> > all.
> > 
> > This series updates all three drivers so that they use the recently
> > merged QCryptoSecret API for getting passwords. Each driver gains
> > a 'passwordid' property that can be set to provide the ID of a
> > QCryptoSecret object instance, which in turn provides the actual
> > password data.
> > 
> > This series is required in order to fix a long standing CVE security
> > flaw in libvirt, whereby passwords are exposed in the command line
> > arguments and so visible in process listing
> > 
> > This series would benefit from the --object additions to qemu-img,
> > qemu-io and qemu-nbd, but this is not a pre-requisite for its merge
> > as it us still useful in the system emulator without that support:
> > 
> >   https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03381.html
> > 
> > Changed in v3:
> > 
> >  - Rename 'passwordid' to 'password-id', 'proxypasswordid'
> >    to 'proxy-password-id' and 'proxyusername' to 'proxy-username'
> >    (Markus)
> > 
> > Daniel P. Berrange (3):
> >   rbd: add support for getting password from QCryptoSecret object
> >   curl: add support for HTTP authentication parameters
> >   iscsi: add support for getting CHAP password via QCryptoSecret API
> > 
> >  block/curl.c  | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  block/iscsi.c | 24 +++++++++++++++++++++-
> >  block/rbd.c   | 47 ++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 136 insertions(+), 1 deletion(-)
> > 
> 
> Apologizing in advance for bikeshedding: what about using proxy-secret
> and secret instead?  Traditionally the name of object options has
> referred to the name of the class.

I wanted to avoid using the word 'secret', because in the future when we
have ability to run LUKS encryption over any backend, we will have need
to pass multiple secrets for a single drive spec. For example, we'll need
one secret to provide the RBD password, and one secret to provide the LUKS
decryption passphrase. So I felt using 'password' is a better choice to
standardize on for the protocol authentication needs.

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

Re: [Qemu-devel] [PATCH v3 0/3] Use QCryptoSecret for block device passwords

[Paolo Bonzini]

On 19/01/2016 17:46, Daniel P. Berrange wrote:
> On Tue, Jan 19, 2016 at 05:32:35PM +0100, Paolo Bonzini wrote:
>>
>>
>> On 19/01/2016 14:51, Daniel P. Berrange wrote:
>>> This series was previously posted:
>>>
>>>   v1: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html
>>>   v2: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg03809.html
>>>
>>> The RBD, Curl and iSCSI block device drivers all need the ability
>>> to accept a password to authenticate with the remote network storage
>>> server. Currently RBD and iSCSI both just take the password in clear
>>> text as part of the block parameters which is insecure (passwords are
>>> visible in the process listing), while Curl doesn't support auth at
>>> all.
>>>
>>> This series updates all three drivers so that they use the recently
>>> merged QCryptoSecret API for getting passwords. Each driver gains
>>> a 'passwordid' property that can be set to provide the ID of a
>>> QCryptoSecret object instance, which in turn provides the actual
>>> password data.
>>>
>>> This series is required in order to fix a long standing CVE security
>>> flaw in libvirt, whereby passwords are exposed in the command line
>>> arguments and so visible in process listing
>>>
>>> This series would benefit from the --object additions to qemu-img,
>>> qemu-io and qemu-nbd, but this is not a pre-requisite for its merge
>>> as it us still useful in the system emulator without that support:
>>>
>>>   https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03381.html
>>>
>>> Changed in v3:
>>>
>>>  - Rename 'passwordid' to 'password-id', 'proxypasswordid'
>>>    to 'proxy-password-id' and 'proxyusername' to 'proxy-username'
>>>    (Markus)
>>>
>>> Daniel P. Berrange (3):
>>>   rbd: add support for getting password from QCryptoSecret object
>>>   curl: add support for HTTP authentication parameters
>>>   iscsi: add support for getting CHAP password via QCryptoSecret API
>>>
>>>  block/curl.c  | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  block/iscsi.c | 24 +++++++++++++++++++++-
>>>  block/rbd.c   | 47 ++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 136 insertions(+), 1 deletion(-)
>>>
>>
>> Apologizing in advance for bikeshedding: what about using proxy-secret
>> and secret instead?  Traditionally the name of object options has
>> referred to the name of the class.
> 
> I wanted to avoid using the word 'secret', because in the future when we
> have ability to run LUKS encryption over any backend, we will have need
> to pass multiple secrets for a single drive spec. For example, we'll need
> one secret to provide the RBD password, and one secret to provide the LUKS
> decryption passphrase. So I felt using 'password' is a better choice to
> standardize on for the protocol authentication needs.

If you have a qcow2->luks->rbd tree, the LUKS passphrase would be
file.secret, while the rbd credentials would be file.file.username and
file.file.secret.

password-secret and proxy-password-secret are fine too.  I just don't
like "id" too much.

Paolo

Re: [Qemu-devel] [PATCH v3 0/3] Use QCryptoSecret for block device passwords

[Daniel P. Berrange]

On Tue, Jan 19, 2016 at 09:41:31PM +0100, Paolo Bonzini wrote:
> 
> 
> On 19/01/2016 17:46, Daniel P. Berrange wrote:
> > On Tue, Jan 19, 2016 at 05:32:35PM +0100, Paolo Bonzini wrote:
> >>
> >>
> >> On 19/01/2016 14:51, Daniel P. Berrange wrote:
> >>> This series was previously posted:
> >>>
> >>>   v1: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html
> >>>   v2: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg03809.html
> >>>
> >>> The RBD, Curl and iSCSI block device drivers all need the ability
> >>> to accept a password to authenticate with the remote network storage
> >>> server. Currently RBD and iSCSI both just take the password in clear
> >>> text as part of the block parameters which is insecure (passwords are
> >>> visible in the process listing), while Curl doesn't support auth at
> >>> all.
> >>>
> >>> This series updates all three drivers so that they use the recently
> >>> merged QCryptoSecret API for getting passwords. Each driver gains
> >>> a 'passwordid' property that can be set to provide the ID of a
> >>> QCryptoSecret object instance, which in turn provides the actual
> >>> password data.
> >>>
> >>> This series is required in order to fix a long standing CVE security
> >>> flaw in libvirt, whereby passwords are exposed in the command line
> >>> arguments and so visible in process listing
> >>>
> >>> This series would benefit from the --object additions to qemu-img,
> >>> qemu-io and qemu-nbd, but this is not a pre-requisite for its merge
> >>> as it us still useful in the system emulator without that support:
> >>>
> >>>   https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03381.html
> >>>
> >>> Changed in v3:
> >>>
> >>>  - Rename 'passwordid' to 'password-id', 'proxypasswordid'
> >>>    to 'proxy-password-id' and 'proxyusername' to 'proxy-username'
> >>>    (Markus)
> >>>
> >>> Daniel P. Berrange (3):
> >>>   rbd: add support for getting password from QCryptoSecret object
> >>>   curl: add support for HTTP authentication parameters
> >>>   iscsi: add support for getting CHAP password via QCryptoSecret API
> >>>
> >>>  block/curl.c  | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>  block/iscsi.c | 24 +++++++++++++++++++++-
> >>>  block/rbd.c   | 47 ++++++++++++++++++++++++++++++++++++++++++
> >>>  3 files changed, 136 insertions(+), 1 deletion(-)
> >>>
> >>
> >> Apologizing in advance for bikeshedding: what about using proxy-secret
> >> and secret instead?  Traditionally the name of object options has
> >> referred to the name of the class.
> > 
> > I wanted to avoid using the word 'secret', because in the future when we
> > have ability to run LUKS encryption over any backend, we will have need
> > to pass multiple secrets for a single drive spec. For example, we'll need
> > one secret to provide the RBD password, and one secret to provide the LUKS
> > decryption passphrase. So I felt using 'password' is a better choice to
> > standardize on for the protocol authentication needs.
> 
> If you have a qcow2->luks->rbd tree, the LUKS passphrase would be
> file.secret, while the rbd credentials would be file.file.username and
> file.file.secret.

Yep, I just feel that could be slightly confusing as to which is
which if they have the same name.

> password-secret and proxy-password-secret are fine too.  I just don't
> like "id" too much.

That sounds ok to me.

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