All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] block: curl: Allow passing cookies via QCryptoSecret
@ 2017-05-04 14:00 Peter Krempa
  2017-05-04 14:22 ` Eric Blake
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Peter Krempa @ 2017-05-04 14:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, Max Reitz, berrange, 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 <pkrempa@redhat.com>
---
 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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH] block: curl: Allow passing cookies via QCryptoSecret
  2017-05-04 14:00 [Qemu-devel] [PATCH] block: curl: Allow passing cookies via QCryptoSecret Peter Krempa
@ 2017-05-04 14:22 ` Eric Blake
  2017-05-04 14:34 ` Daniel P. Berrange
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Eric Blake @ 2017-05-04 14:22 UTC (permalink / raw)
  To: Peter Krempa, qemu-devel; +Cc: qemu-block, Max Reitz, Daniel P. Berrange

[-- Attachment #1: Type: text/plain, Size: 1273 bytes --]

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

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH] block: curl: Allow passing cookies via QCryptoSecret
  2017-05-04 14:00 [Qemu-devel] [PATCH] block: curl: Allow passing cookies via QCryptoSecret Peter Krempa
  2017-05-04 14:22 ` Eric Blake
@ 2017-05-04 14:34 ` Daniel P. Berrange
  2017-05-09 15:06 ` [Qemu-devel] [Qemu-block] " Kevin Wolf
  2017-05-09 15:44 ` Jeff Cody
  3 siblings, 0 replies; 8+ messages in thread
From: Daniel P. Berrange @ 2017-05-04 14:34 UTC (permalink / raw)
  To: Peter Krempa; +Cc: qemu-devel, qemu-block, Max Reitz

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

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [Qemu-block] [PATCH] block: curl: Allow passing cookies via QCryptoSecret
  2017-05-04 14:00 [Qemu-devel] [PATCH] block: curl: Allow passing cookies via QCryptoSecret Peter Krempa
  2017-05-04 14:22 ` Eric Blake
  2017-05-04 14:34 ` Daniel P. Berrange
@ 2017-05-09 15:06 ` Kevin Wolf
  2017-05-09 15:44 ` Jeff Cody
  3 siblings, 0 replies; 8+ messages in thread
From: Kevin Wolf @ 2017-05-09 15:06 UTC (permalink / raw)
  To: Peter Krempa; +Cc: qemu-devel, berrange, qemu-block, Max Reitz, jcody

Am 04.05.2017 um 16:00 hat Peter Krempa geschrieben:
> 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 <pkrempa@redhat.com>

I didn't really look at the patch, just adding a maintainer CC:

$ scripts/get_maintainer.pl -f block/curl.c
Jeff Cody <jcody@redhat.com> (supporter:CURL)
...

Kevin

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [Qemu-block] [PATCH] block: curl: Allow passing cookies via QCryptoSecret
  2017-05-04 14:00 [Qemu-devel] [PATCH] block: curl: Allow passing cookies via QCryptoSecret Peter Krempa
                   ` (2 preceding siblings ...)
  2017-05-09 15:06 ` [Qemu-devel] [Qemu-block] " Kevin Wolf
@ 2017-05-09 15:44 ` Jeff Cody
  2017-05-09 19:43   ` Manos Pitsidianakis
  3 siblings, 1 reply; 8+ messages in thread
From: Jeff Cody @ 2017-05-09 15:44 UTC (permalink / raw)
  To: Peter Krempa; +Cc: qemu-devel, berrange, qemu-block, Max Reitz

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

Thanks,

Reviewed-by: Jeff Cody <jcody@redhat.com>

Also:

Applied to my block branch:

git://github.com/codyprime/qemu-kvm-jtc block

-Jeff

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [Qemu-block] [PATCH] block: curl: Allow passing cookies via QCryptoSecret
  2017-05-09 15:44 ` Jeff Cody
@ 2017-05-09 19:43   ` Manos Pitsidianakis
  2017-05-09 19:52     ` Eric Blake
  0 siblings, 1 reply; 8+ messages in thread
From: Manos Pitsidianakis @ 2017-05-09 19:43 UTC (permalink / raw)
  To: qemu-block, qemu-devel

On Thu, May 04, 2017 at 04:00:06PM +0200, Peter Krempa wrote:
> +    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);
> +    }

There's no check here for if both cookie and cookie_secret are NULL. 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [Qemu-block] [PATCH] block: curl: Allow passing cookies via QCryptoSecret
  2017-05-09 19:43   ` Manos Pitsidianakis
@ 2017-05-09 19:52     ` Eric Blake
  2017-05-09 20:03       ` Manos Pitsidianakis
  0 siblings, 1 reply; 8+ messages in thread
From: Eric Blake @ 2017-05-09 19:52 UTC (permalink / raw)
  To: Manos Pitsidianakis, qemu-block, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1015 bytes --]

On 05/09/2017 02:43 PM, Manos Pitsidianakis wrote:
> On Thu, May 04, 2017 at 04:00:06PM +0200, Peter Krempa wrote:
>> +    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);
>> +    }
> 
> There's no check here for if both cookie and cookie_secret are NULL.

Is that a problem?  s->cookie ends up as NULL (thanks to g_strdup()
semantics), which merely means there's no cookie to be sent after all.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [Qemu-block] [PATCH] block: curl: Allow passing cookies via QCryptoSecret
  2017-05-09 19:52     ` Eric Blake
@ 2017-05-09 20:03       ` Manos Pitsidianakis
  0 siblings, 0 replies; 8+ messages in thread
From: Manos Pitsidianakis @ 2017-05-09 20:03 UTC (permalink / raw)
  To: qemu-devel, qemu-block

[-- Attachment #1: Type: text/plain, Size: 1175 bytes --]

On Tue, May 09, 2017 at 02:52:38PM -0500, Eric Blake wrote:
>On 05/09/2017 02:43 PM, Manos Pitsidianakis wrote:
>> On Thu, May 04, 2017 at 04:00:06PM +0200, Peter Krempa wrote:
>>> +    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);
>>> +    }
>>
>> There's no check here for if both cookie and cookie_secret are NULL.
>
>Is that a problem?  s->cookie ends up as NULL (thanks to g_strdup()
>semantics), which merely means there's no cookie to be sent after all.

Ah yes, g_strdup(NULL) returns NULL. Apologies for the noise.


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



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-05-09 20:03 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-04 14:00 [Qemu-devel] [PATCH] block: curl: Allow passing cookies via QCryptoSecret Peter Krempa
2017-05-04 14:22 ` Eric Blake
2017-05-04 14:34 ` Daniel P. Berrange
2017-05-09 15:06 ` [Qemu-devel] [Qemu-block] " Kevin Wolf
2017-05-09 15:44 ` Jeff Cody
2017-05-09 19:43   ` Manos Pitsidianakis
2017-05-09 19:52     ` Eric Blake
2017-05-09 20:03       ` Manos Pitsidianakis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.