* [Qemu-devel] [PATCH v4 1/3] rbd: add support for getting password from QCryptoSecret object
2016-01-21 14:19 [Qemu-devel] [PATCH v4 0/3] Use QCryptoSecret for block device passwords Daniel P. Berrange
@ 2016-01-21 14:19 ` Daniel P. Berrange
2016-01-21 14:19 ` [Qemu-devel] [PATCH v4 2/3] curl: add support for HTTP authentication parameters Daniel P. Berrange
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrange @ 2016-01-21 14:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster, qemu-block
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-secret' 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-secret=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 51b64f3..abfea61 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 *secretid,
+ Error **errp)
+{
+ if (secretid == 0) {
+ return 0;
+ }
+
+ gchar *secret = qcrypto_secret_lookup_as_base64(secretid,
+ 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 *secretid;
rados_t cluster;
rados_ioctx_t io_ctx;
int ret;
+ secretid = qemu_opt_get(opts, "password-secret");
+
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, secretid, 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-secret",
+ .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 *secretid;
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");
+ secretid = qemu_opt_get(opts, "password-secret");
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, secretid, 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-secret",
+ .type = QEMU_OPT_STRING,
+ .help = "ID of secret providing the password",
+ },
{ /* end of list */ }
}
};
--
2.5.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v4 2/3] curl: add support for HTTP authentication parameters
2016-01-21 14:19 [Qemu-devel] [PATCH v4 0/3] Use QCryptoSecret for block device passwords Daniel P. Berrange
2016-01-21 14:19 ` [Qemu-devel] [PATCH v4 1/3] rbd: add support for getting password from QCryptoSecret object Daniel P. Berrange
@ 2016-01-21 14:19 ` Daniel P. Berrange
2016-01-21 14:19 ` [Qemu-devel] [PATCH v4 3/3] iscsi: add support for getting CHAP password via QCryptoSecret API Daniel P. Berrange
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrange @ 2016-01-21 14:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster, qemu-block
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-secret', 'proxy-username'
and 'proxy-password-secret'. 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-secret=sec0,\
proxy-username=dan,proxy-password-secret=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 1507e0a..c70bfb4 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -27,6 +27,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
@@ -78,6 +79,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_PASSWORD_SECRET "password-secret"
+#define CURL_BLOCK_OPT_PROXY_USERNAME "proxy-username"
+#define CURL_BLOCK_OPT_PROXY_PASSWORD_SECRET "proxy-password-secret"
struct BDRVCURLState;
@@ -120,6 +125,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);
@@ -419,6 +428,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.
@@ -525,10 +549,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_PASSWORD_SECRET,
+ .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_PASSWORD_SECRET,
+ .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)
{
@@ -539,6 +584,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
const char *file;
const char *cookie;
double d;
+ const char *secretid;
static int inited = 0;
@@ -580,6 +626,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));
+ secretid = qemu_opt_get(opts, CURL_BLOCK_OPT_PASSWORD_SECRET);
+
+ if (secretid) {
+ s->password = qcrypto_secret_lookup_as_utf8(secretid, errp);
+ if (!s->password) {
+ goto out_noclean;
+ }
+ }
+
+ s->proxyusername = g_strdup(
+ qemu_opt_get(opts, CURL_BLOCK_OPT_PROXY_USERNAME));
+ secretid = qemu_opt_get(opts, CURL_BLOCK_OPT_PROXY_PASSWORD_SECRET);
+ if (secretid) {
+ s->proxypassword = qcrypto_secret_lookup_as_utf8(secretid, 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] 6+ messages in thread
* [Qemu-devel] [PATCH v4 3/3] iscsi: add support for getting CHAP password via QCryptoSecret API
2016-01-21 14:19 [Qemu-devel] [PATCH v4 0/3] Use QCryptoSecret for block device passwords Daniel P. Berrange
2016-01-21 14:19 ` [Qemu-devel] [PATCH v4 1/3] rbd: add support for getting password from QCryptoSecret object Daniel P. Berrange
2016-01-21 14:19 ` [Qemu-devel] [PATCH v4 2/3] curl: add support for HTTP authentication parameters Daniel P. Berrange
@ 2016-01-21 14:19 ` Daniel P. Berrange
2016-02-04 15:56 ` [Qemu-devel] [PATCH v4 0/3] Use QCryptoSecret for block device passwords Paolo Bonzini
2016-02-08 3:52 ` [Qemu-devel] [Qemu-block] " Jeff Cody
4 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrange @ 2016-01-21 14:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster, qemu-block
The iSCSI driver currently accepts the CHAP password in plain text
as a block driver property. This change adds a new "password-secret"
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-secret=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 bffd707..c22d8a2 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 *secretid;
+ 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;
}
+ secretid = qemu_opt_get(opts, "password-secret");
password = qemu_opt_get(opts, "password");
- if (!password) {
+ if (secretid && password) {
+ error_setg(errp, "'password' and 'password-secret' properties are "
+ "mutually exclusive");
+ return;
+ }
+ if (secretid) {
+ secret = qcrypto_secret_lookup_as_utf8(secretid, 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-secret",
+ .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] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v4 0/3] Use QCryptoSecret for block device passwords
2016-01-21 14:19 [Qemu-devel] [PATCH v4 0/3] Use QCryptoSecret for block device passwords Daniel P. Berrange
` (2 preceding siblings ...)
2016-01-21 14:19 ` [Qemu-devel] [PATCH v4 3/3] iscsi: add support for getting CHAP password via QCryptoSecret API Daniel P. Berrange
@ 2016-02-04 15:56 ` Paolo Bonzini
2016-02-08 3:52 ` [Qemu-devel] [Qemu-block] " Jeff Cody
4 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2016-02-04 15:56 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel; +Cc: Jeff Cody, Markus Armbruster, qemu-block
On 21/01/2016 15:19, 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
> v3: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03461.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 v4:
>
> - Rename 'password-id' to 'password-secret', 'proxy-password-id'
> to 'proxy-password-secret' (Paolo)
>
> 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(-)
>
Ping?
Jeff, these three are all network-based block device drivers so I think
you can apply the whole series.
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v4 0/3] Use QCryptoSecret for block device passwords
2016-01-21 14:19 [Qemu-devel] [PATCH v4 0/3] Use QCryptoSecret for block device passwords Daniel P. Berrange
` (3 preceding siblings ...)
2016-02-04 15:56 ` [Qemu-devel] [PATCH v4 0/3] Use QCryptoSecret for block device passwords Paolo Bonzini
@ 2016-02-08 3:52 ` Jeff Cody
4 siblings, 0 replies; 6+ messages in thread
From: Jeff Cody @ 2016-02-08 3:52 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: Paolo Bonzini, qemu-devel, qemu-block, Markus Armbruster
On Thu, Jan 21, 2016 at 02:19:18PM +0000, 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
> v3: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03461.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 v4:
>
> - Rename 'password-id' to 'password-secret', 'proxy-password-id'
> to 'proxy-password-secret' (Paolo)
>
> 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
>
>
Thanks, applied to my git branch:
git://github.com/codyprime/qemu-kvm-jtc.git block
-Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread