subject:"\[Qemu\-devel\] \[PATCH\] block\: curl\: Allow passing cookies via QCryptoSecret"

[Qemu-devel] [PATCH] block: curl: Allow passing cookies via QCryptoSecret

2017-05-04 Thread Peter Krempa

Since cookies can contain sensitive data (session ID, etc ...) it is
desired to hide them from the prying eyes of users. Add a possibility to
pass them via the secret infrastructure.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1447413

Signed-off-by: Peter Krempa 
---
 block/curl.c | 24 +++-
 qapi/block-core.json | 12 ++--
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index 2708d57c2f..483640b14a 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -85,6 +85,7 @@ 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_COOKIE_SECRET "cookie-secret"
 #define CURL_BLOCK_OPT_USERNAME "username"
 #define CURL_BLOCK_OPT_PASSWORD_SECRET "password-secret"
 #define CURL_BLOCK_OPT_PROXY_USERNAME "proxy-username"
@@ -624,6 +625,11 @@ static QemuOptsList runtime_opts = {
 .help = "Pass the cookie or list of cookies with each request"
 },
 {
+.name = CURL_BLOCK_OPT_COOKIE_SECRET,
+.type = QEMU_OPT_STRING,
+.help = "ID of secret used as cookie passed with each request"
+},
+{
 .name = CURL_BLOCK_OPT_USERNAME,
 .type = QEMU_OPT_STRING,
 .help = "Username for HTTP auth"
@@ -657,6 +663,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, 
int flags,
 Error *local_err = NULL;
 const char *file;
 const char *cookie;
+const char *cookie_secret;
 double d;
 const char *secretid;
 const char *protocol_delimiter;
@@ -693,7 +700,22 @@ static int curl_open(BlockDriverState *bs, QDict *options, 
int flags,
 s->sslverify = qemu_opt_get_bool(opts, CURL_BLOCK_OPT_SSLVERIFY, true);

 cookie = qemu_opt_get(opts, CURL_BLOCK_OPT_COOKIE);
-s->cookie = g_strdup(cookie);
+cookie_secret = qemu_opt_get(opts, CURL_BLOCK_OPT_COOKIE_SECRET);
+
+if (cookie && cookie_secret) {
+error_setg(errp,
+   "curl driver cannot handle both cookie and cookie secret");
+goto out_noclean;
+}
+
+if (cookie_secret) {
+s->cookie = qcrypto_secret_lookup_as_utf8(cookie_secret, errp);
+if (!s->cookie) {
+goto out_noclean;
+}
+} else {
+s->cookie = g_strdup(cookie);
+}

 file = qemu_opt_get(opts, CURL_BLOCK_OPT_URL);
 if (file == NULL) {
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 87fb747ab6..b1643d2032 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2782,11 +2782,15 @@
 #   "name1=content1; name2=content2;" as explained by
 #   CURLOPT_COOKIE(3). Defaults to no cookies.
 #
+# @cookie-secret: ID of a QCryptoSecret object providing the cookie data in a
+# secure way. See @cookie for the format. (since 2.10)
+#
 # Since: 2.9
 ##
 { 'struct': 'BlockdevOptionsCurlHttp',
   'base': 'BlockdevOptionsCurlBase',
-  'data': { '*cookie': 'str' } }
+  'data': { '*cookie': 'str',
+'*cookie-secret': 'str'} }

 ##
 # @BlockdevOptionsCurlHttps:
@@ -2801,12 +2805,16 @@
 # @sslverify:   Whether to verify the SSL certificate's validity (defaults to
 #   true)
 #
+# @cookie-secret: ID of a QCryptoSecret object providing the cookie data in a
+# secure way. See @cookie for the format. (since 2.10)
+#
 # Since: 2.9
 ##
 { 'struct': 'BlockdevOptionsCurlHttps',
   'base': 'BlockdevOptionsCurlBase',
   'data': { '*cookie': 'str',
-'*sslverify': 'bool' } }
+'*sslverify': 'bool',
+'*cookie-secret': 'str'} }

 ##
 # @BlockdevOptionsCurlFtp:
-- 
2.12.2

Re: [Qemu-devel] [PATCH] block: curl: Allow passing cookies via QCryptoSecret

2017-05-04 Thread Eric Blake

On 05/04/2017 09:00 AM, Peter Krempa wrote:
> Since cookies can contain sensitive data (session ID, etc ...) it is
> desired to hide them from the prying eyes of users. Add a possibility to
> pass them via the secret infrastructure.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1447413
> 
> Signed-off-by: Peter Krempa 
> ---
>  block/curl.c | 24 +++-
>  qapi/block-core.json | 12 ++--
>  2 files changed, 33 insertions(+), 3 deletions(-)
> 

> +if (cookie_secret) {
> +s->cookie = qcrypto_secret_lookup_as_utf8(cookie_secret, errp);
> +if (!s->cookie) {
> +goto out_noclean;
> +}

Can s->cookie ever be exposed back to the user (such as via a
query-block command)?  If so, we should rather store cookie_secret for
display to the user, rather than the decoded version.

But I couldn't see where we would expose it, so I think you are safe.
I'd wait for another review, probably from Dan since he is the
secret-object expert, but I'm comfortable if you add:

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] block: curl: Allow passing cookies via QCryptoSecret

2017-05-04 Thread Daniel P. Berrange

On Thu, May 04, 2017 at 04:00:06PM +0200, Peter Krempa wrote:
> Since cookies can contain sensitive data (session ID, etc ...) it is
> desired to hide them from the prying eyes of users. Add a possibility to
> pass them via the secret infrastructure.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1447413
> 
> Signed-off-by: Peter Krempa 
> ---
>  block/curl.c | 24 +++-
>  qapi/block-core.json | 12 ++--
>  2 files changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/block/curl.c b/block/curl.c
> index 2708d57c2f..483640b14a 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -85,6 +85,7 @@ 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_COOKIE_SECRET "cookie-secret"
>  #define CURL_BLOCK_OPT_USERNAME "username"
>  #define CURL_BLOCK_OPT_PASSWORD_SECRET "password-secret"
>  #define CURL_BLOCK_OPT_PROXY_USERNAME "proxy-username"
> @@ -624,6 +625,11 @@ static QemuOptsList runtime_opts = {
>  .help = "Pass the cookie or list of cookies with each request"
>  },
>  {
> +.name = CURL_BLOCK_OPT_COOKIE_SECRET,
> +.type = QEMU_OPT_STRING,
> +.help = "ID of secret used as cookie passed with each request"
> +},
> +{
>  .name = CURL_BLOCK_OPT_USERNAME,
>  .type = QEMU_OPT_STRING,
>  .help = "Username for HTTP auth"
> @@ -657,6 +663,7 @@ static int curl_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  Error *local_err = NULL;
>  const char *file;
>  const char *cookie;
> +const char *cookie_secret;
>  double d;
>  const char *secretid;
>  const char *protocol_delimiter;
> @@ -693,7 +700,22 @@ static int curl_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  s->sslverify = qemu_opt_get_bool(opts, CURL_BLOCK_OPT_SSLVERIFY, true);
> 
>  cookie = qemu_opt_get(opts, CURL_BLOCK_OPT_COOKIE);
> -s->cookie = g_strdup(cookie);
> +cookie_secret = qemu_opt_get(opts, CURL_BLOCK_OPT_COOKIE_SECRET);
> +
> +if (cookie && cookie_secret) {
> +error_setg(errp,
> +   "curl driver cannot handle both cookie and cookie 
> secret");
> +goto out_noclean;
> +}
> +
> +if (cookie_secret) {
> +s->cookie = qcrypto_secret_lookup_as_utf8(cookie_secret, errp);
> +if (!s->cookie) {
> +goto out_noclean;
> +}
> +} else {
> +s->cookie = g_strdup(cookie);
> +}
> 
>  file = qemu_opt_get(opts, CURL_BLOCK_OPT_URL);
>  if (file == NULL) {
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 87fb747ab6..b1643d2032 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2782,11 +2782,15 @@
>  #   "name1=content1; name2=content2;" as explained by
>  #   CURLOPT_COOKIE(3). Defaults to no cookies.
>  #
> +# @cookie-secret: ID of a QCryptoSecret object providing the cookie data in a
> +# secure way. See @cookie for the format. (since 2.10)
> +#
>  # Since: 2.9
>  ##
>  { 'struct': 'BlockdevOptionsCurlHttp',
>'base': 'BlockdevOptionsCurlBase',
> -  'data': { '*cookie': 'str' } }
> +  'data': { '*cookie': 'str',
> +'*cookie-secret': 'str'} }
> 
>  ##
>  # @BlockdevOptionsCurlHttps:
> @@ -2801,12 +2805,16 @@
>  # @sslverify:   Whether to verify the SSL certificate's validity (defaults to
>  #   true)
>  #
> +# @cookie-secret: ID of a QCryptoSecret object providing the cookie data in a
> +# secure way. See @cookie for the format. (since 2.10)
> +#
>  # Since: 2.9
>  ##
>  { 'struct': 'BlockdevOptionsCurlHttps',
>'base': 'BlockdevOptionsCurlBase',
>'data': { '*cookie': 'str',
> -'*sslverify': 'bool' } }
> +'*sslverify': 'bool',
> +'*cookie-secret': 'str'} }
> 
>  ##
>  # @BlockdevOptionsCurlFtp:

This proposed approach for 'cookie-secret' is consistent with how we deal
with the existing 'cookie' parameter (even though that is itself somewhat
unpleasantly designed for QAPI).

 Reviewed-by: Daniel P. Berrange 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[Qemu-devel] [PATCH] block: curl: Allow passing cookies via QCryptoSecret

Re: [Qemu-devel] [PATCH] block: curl: Allow passing cookies via QCryptoSecret

Re: [Qemu-devel] [PATCH] block: curl: Allow passing cookies via QCryptoSecret

3 matches

Site Navigation

Mail list logo

Footer information