1. Patchew
  2. QEMU
  3. View series

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

Peter Krempa posted 1 patch 6 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/f4a22cdebdd0bca6a13a43a2a6deead7f2ec4bb3.1493906281.git.pkrempa@redhat.com
Test checkpatch passed
Test docker passed
Test s390x passed
block/curl.c         | 24 +++++++++++++++++++++++-
qapi/block-core.json | 12 ++++++++++--
2 files changed, 33 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH] block: curl: Allow passing cookies via QCryptoSecret
Posted by Peter Krempa 6 years, 11 months ago 
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
Re: [Qemu-devel] [PATCH] block: curl: Allow passing cookies via QCryptoSecret
Posted by Eric Blake 6 years, 11 months ago 
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
Re: [Qemu-devel] [PATCH] block: curl: Allow passing cookies via QCryptoSecret
Posted by Daniel P. Berrange 6 years, 11 months ago 
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 :|
Re: [Qemu-devel] [Qemu-block] [PATCH] block: curl: Allow passing cookies via QCryptoSecret
Posted by Kevin Wolf 6 years, 11 months ago 
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
> 
>
Re: [Qemu-devel] [Qemu-block] [PATCH] block: curl: Allow passing cookies via QCryptoSecret
Posted by Jeff Cody 6 years, 11 months ago 
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
Re: [Qemu-devel] [Qemu-block] [PATCH] block: curl: Allow passing cookies via QCryptoSecret
Posted by Manos Pitsidianakis 6 years, 11 months ago 
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.
Re: [Qemu-devel] [Qemu-block] [PATCH] block: curl: Allow passing cookies via QCryptoSecret
Posted by Eric Blake 6 years, 11 months ago 
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
Re: [Qemu-devel] [Qemu-block] [PATCH] block: curl: Allow passing cookies via QCryptoSecret
Posted by Manos Pitsidianakis 6 years, 11 months ago 
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
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.