* [Qemu-devel] [PATCH v2 1/3] rbd: add support for getting password from QCryptoSecret object
2015-12-21 14:59 [Qemu-devel] [PATCH v2 0/3] Use QCryptoSecret for block device passwords Daniel P. Berrange
@ 2015-12-21 14:59 ` Daniel P. Berrange
2015-12-21 15:57 ` Josh Durgin
2015-12-21 14:59 ` [Qemu-devel] [PATCH v2 2/3] curl: add support for HTTP authentication parameters Daniel P. Berrange
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Daniel P. Berrange @ 2015-12-21 14:59 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Josh Durgin, qemu-block, Jeff Cody, Ronnie Sahlberg,
Paolo Bonzini
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(+)
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 */ }
}
};
--
2.5.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/3] rbd: add support for getting password from QCryptoSecret object
2015-12-21 14:59 ` [Qemu-devel] [PATCH v2 1/3] rbd: add support for getting password from QCryptoSecret object Daniel P. Berrange
@ 2015-12-21 15:57 ` Josh Durgin
0 siblings, 0 replies; 9+ messages in thread
From: Josh Durgin @ 2015-12-21 15:57 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Kevin Wolf, Paolo Bonzini, Jeff Cody, qemu-block, 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 <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 */ }
> }
> };
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v2 2/3] curl: add support for HTTP authentication parameters
2015-12-21 14:59 [Qemu-devel] [PATCH v2 0/3] Use QCryptoSecret for block device passwords Daniel P. Berrange
2015-12-21 14:59 ` [Qemu-devel] [PATCH v2 1/3] rbd: add support for getting password from QCryptoSecret object Daniel P. Berrange
@ 2015-12-21 14:59 ` Daniel P. Berrange
2015-12-21 14:59 ` [Qemu-devel] [PATCH v2 3/3] iscsi: add support for getting CHAP password via QCryptoSecret API Daniel P. Berrange
2016-01-11 15:12 ` [Qemu-devel] [PATCH v2 0/3] Use QCryptoSecret for block device passwords Markus Armbruster
3 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Berrange @ 2015-12-21 14:59 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Josh Durgin, qemu-block, Jeff Cody, Ronnie Sahlberg,
Paolo Bonzini
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, passwordid, proxyusername and
proxypasswordid. 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,passwordid=sec0,\
proxyusername=dan,passwordid=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..0552abf 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 "passwordid"
+#define CURL_BLOCK_OPT_PROXY_USERNAME "proxyusername"
+#define CURL_BLOCK_OPT_PROXY_PASSWORDID "proxypasswordid"
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH v2 3/3] iscsi: add support for getting CHAP password via QCryptoSecret API
2015-12-21 14:59 [Qemu-devel] [PATCH v2 0/3] Use QCryptoSecret for block device passwords Daniel P. Berrange
2015-12-21 14:59 ` [Qemu-devel] [PATCH v2 1/3] rbd: add support for getting password from QCryptoSecret object Daniel P. Berrange
2015-12-21 14:59 ` [Qemu-devel] [PATCH v2 2/3] curl: add support for HTTP authentication parameters Daniel P. Berrange
@ 2015-12-21 14:59 ` Daniel P. Berrange
2015-12-21 15:58 ` Paolo Bonzini
2016-01-11 15:12 ` [Qemu-devel] [PATCH v2 0/3] Use QCryptoSecret for block device passwords Markus Armbruster
3 siblings, 1 reply; 9+ messages in thread
From: Daniel P. Berrange @ 2015-12-21 14:59 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Josh Durgin, qemu-block, Jeff Cody, Ronnie Sahlberg,
Paolo Bonzini
The iSCSI driver currently accepts the CHAP password in plain text
as a block driver property. This change adds a new "passwordid"
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,passwordid=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 bd1f1bf..96fa3e1 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, "passwordid");
password = qemu_opt_get(opts, "password");
- if (!password) {
+ if (passwordid && password) {
+ error_setg(errp, "'password' and 'passwordid' 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,
@@ -1848,6 +1865,11 @@ static QemuOptsList qemu_iscsi_opts = {
.type = QEMU_OPT_STRING,
.help = "password for CHAP authentication to target",
},{
+ .name = "passwordid",
+ .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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH v2 3/3] iscsi: add support for getting CHAP password via QCryptoSecret API
2015-12-21 14:59 ` [Qemu-devel] [PATCH v2 3/3] iscsi: add support for getting CHAP password via QCryptoSecret API Daniel P. Berrange
@ 2015-12-21 15:58 ` Paolo Bonzini
2015-12-21 16:17 ` Daniel P. Berrange
0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2015-12-21 15:58 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Kevin Wolf, Jeff Cody, qemu-block, Josh Durgin, Ronnie Sahlberg
On 21/12/2015 15:59, Daniel P. Berrange wrote:
> The iSCSI driver currently accepts the CHAP password in plain text
> as a block driver property. This change adds a new "passwordid"
> 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,passwordid=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 bd1f1bf..96fa3e1 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, "passwordid");
> password = qemu_opt_get(opts, "password");
> - if (!password) {
> + if (passwordid && password) {
> + error_setg(errp, "'password' and 'passwordid' properties are "
> + "mutually exclusive");
> + return;
> + }
> + if (passwordid) {
> + secret = qcrypto_secret_lookup_as_utf8(passwordid, errp);
I'm not sure about the UTF-8 part (it should be binary), but I think we
discussed this already. Apart from this, the patch is okay.
Paolo
> + 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,
> @@ -1848,6 +1865,11 @@ static QemuOptsList qemu_iscsi_opts = {
> .type = QEMU_OPT_STRING,
> .help = "password for CHAP authentication to target",
> },{
> + .name = "passwordid",
> + .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. "
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH v2 3/3] iscsi: add support for getting CHAP password via QCryptoSecret API
2015-12-21 15:58 ` Paolo Bonzini
@ 2015-12-21 16:17 ` Daniel P. Berrange
0 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Berrange @ 2015-12-21 16:17 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Kevin Wolf, Josh Durgin, qemu-block, Jeff Cody, qemu-devel,
Ronnie Sahlberg
On Mon, Dec 21, 2015 at 04:58:41PM +0100, Paolo Bonzini wrote:
>
>
> On 21/12/2015 15:59, Daniel P. Berrange wrote:
> > The iSCSI driver currently accepts the CHAP password in plain text
> > as a block driver property. This change adds a new "passwordid"
> > 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,passwordid=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 bd1f1bf..96fa3e1 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, "passwordid");
> > password = qemu_opt_get(opts, "password");
> > - if (!password) {
> > + if (passwordid && password) {
> > + error_setg(errp, "'password' and 'passwordid' properties are "
> > + "mutually exclusive");
> > + return;
> > + }
> > + if (passwordid) {
> > + secret = qcrypto_secret_lookup_as_utf8(passwordid, errp);
>
> I'm not sure about the UTF-8 part (it should be binary), but I think we
> discussed this already. Apart from this, the patch is okay.
The password is passed into libiscsi using
iscsi_set_initiator_username_pwd()
which expects a NULL terminated string. This gives us a choice of
clamping the data to 7-bit ascii only, or using utf-8. We can't
pass it 8-bit data, as that can contain embedded NULs. So IIUC
using utf-8 is best thing here.
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 :|
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] Use QCryptoSecret for block device passwords
2015-12-21 14:59 [Qemu-devel] [PATCH v2 0/3] Use QCryptoSecret for block device passwords Daniel P. Berrange
` (2 preceding siblings ...)
2015-12-21 14:59 ` [Qemu-devel] [PATCH v2 3/3] iscsi: add support for getting CHAP password via QCryptoSecret API Daniel P. Berrange
@ 2016-01-11 15:12 ` Markus Armbruster
2016-01-11 15:28 ` Daniel P. Berrange
3 siblings, 1 reply; 9+ messages in thread
From: Markus Armbruster @ 2016-01-11 15:12 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: Kevin Wolf, Josh Durgin, qemu-block, Jeff Cody, qemu-devel,
Ronnie Sahlberg, Paolo Bonzini
"Daniel P. Berrange" <berrange@redhat.com> writes:
> 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.
I'd find password-id easier to read than passwordid.
> These patches were previously sent as part of a larger series:
>
> https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html
>
> The QCryptoSecret object support was already merged, and the additions
> to qemu-img/qemu-io/qemu-nbd I'll submit separately since there is no
> strict dependancy between those additions and these patches.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH v2 0/3] Use QCryptoSecret for block device passwords
2016-01-11 15:12 ` [Qemu-devel] [PATCH v2 0/3] Use QCryptoSecret for block device passwords Markus Armbruster
@ 2016-01-11 15:28 ` Daniel P. Berrange
0 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Berrange @ 2016-01-11 15:28 UTC (permalink / raw)
To: Markus Armbruster
Cc: Kevin Wolf, Josh Durgin, qemu-block, Jeff Cody, qemu-devel,
Ronnie Sahlberg, Paolo Bonzini
On Mon, Jan 11, 2016 at 04:12:12PM +0100, Markus Armbruster wrote:
> "Daniel P. Berrange" <berrange@redhat.com> writes:
>
> > 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.
>
> I'd find password-id easier to read than passwordid.
Sure, works for me - I'm ambivalent either way, so will update it.
> > These patches were previously sent as part of a larger series:
> >
> > https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html
> >
> > The QCryptoSecret object support was already merged, and the additions
> > to qemu-img/qemu-io/qemu-nbd I'll submit separately since there is no
> > strict dependancy between those additions and these patches.
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 :|
^ permalink raw reply [flat|nested] 9+ messages in thread