GNU bug report logs - #38873
[PATCH] gnu: curl: Make libcurl respect SSL_CERT_{DIR,FILE}

Previous Next

Package: guix-patches;

Reported by: Jakub Kądziołka <kuba <at> kadziolka.net>

Date: Thu, 2 Jan 2020 17:19:02 UTC

Severity: normal

Tags: patch

Done: Marius Bakke <mbakke <at> fastmail.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 38873 in the body.
You can then email your comments to 38873 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#38873; Package guix-patches. (Thu, 02 Jan 2020 17:19:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Jakub Kądziołka <kuba <at> kadziolka.net>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Thu, 02 Jan 2020 17:19:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jakub Kądziołka <kuba <at> kadziolka.net>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: curl: Make libcurl respect SSL_CERT_{DIR,FILE}
Date: Thu, 2 Jan 2020 18:18:26 +0100
* gnu/packages/curl.scm (curl-7.66.0): Use patch.
* gnu/packages/patches/libcurl-use-ssl-cert-env.patch: New file.

This fixes the SSL errors occuring when trying to use rust:cargo's
download functionality.

As an additional advantage, this will probably allow removing some
package-specific work-arounds that have already been made. I have
found such work-arounds in cmake and kodi, but am not familiar enough
with either to confidently remove them.
---
 gnu/packages/curl.scm                         |  4 +-
 .../patches/libcurl-use-ssl-cert-env.patch    | 61 +++++++++++++++++++
 2 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/libcurl-use-ssl-cert-env.patch

diff --git a/gnu/packages/curl.scm b/gnu/packages/curl.scm
index aa5d24c401..c5cd88ec2e 100644
--- a/gnu/packages/curl.scm
+++ b/gnu/packages/curl.scm
@@ -9,6 +9,7 @@
 ;;; Copyright © 2017, 2018 Tobias Geerinckx-Rice <me <at> tobias.gr>
 ;;; Copyright © 2018 Roel Janssen <roel <at> gnu.org>
 ;;; Copyright © 2019 Ricardo Wurmus <rekado <at> elephly.net>
+;;; Copyright © 2020 Jakub Kądziołka <kuba <at> kadziolka.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -153,7 +154,8 @@ tunneling, and so on.")
                                   version ".tar.xz"))
               (sha256
                (base32
-                "1hcqxpibhknhjy56wcxz5vd6m9ggx3ykwp3wp5wx05ih36481d6v"))))))
+                "1hcqxpibhknhjy56wcxz5vd6m9ggx3ykwp3wp5wx05ih36481d6v"))
+              (patches (search-patches "libcurl-use-ssl-cert-env.patch"))))))
 
 (define-public kurly
   (package
diff --git a/gnu/packages/patches/libcurl-use-ssl-cert-env.patch b/gnu/packages/patches/libcurl-use-ssl-cert-env.patch
new file mode 100644
index 0000000000..a68e64adc1
--- /dev/null
+++ b/gnu/packages/patches/libcurl-use-ssl-cert-env.patch
@@ -0,0 +1,61 @@
+Make libcurl respect the SSL_CERT_{DIR,FILE} variables by default. The variables
+are fetched during initialization to preserve thread-safety (curl_global_init(3)
+must be called when no other threads exist).
+===================================================================
+--- curl-7.66.0.orig/lib/easy.c	2020-01-02 15:43:11.883921171 +0100
++++ curl-7.66.0/lib/easy.c	2020-01-02 16:18:54.691882797 +0100
+@@ -134,6 +134,9 @@
+ #  pragma warning(default:4232) /* MSVC extension, dllimport identity */
+ #endif
+ 
++char * Curl_ssl_cert_dir = NULL;
++char * Curl_ssl_cert_file = NULL;
++
+ /**
+  * curl_global_init() globally initializes curl given a bitwise set of the
+  * different features of what to initialize.
+@@ -155,6 +158,9 @@
+ #endif
+   }
+ 
++  Curl_ssl_cert_dir = curl_getenv("SSL_CERT_DIR");
++  Curl_ssl_cert_file = curl_getenv("SSL_CERT_FILE");
++
+   if(!Curl_ssl_init()) {
+     DEBUGF(fprintf(stderr, "Error: Curl_ssl_init failed\n"));
+     return CURLE_FAILED_INIT;
+@@ -260,6 +266,9 @@
+   Curl_ssl_cleanup();
+   Curl_resolver_global_cleanup();
+ 
++  free(Curl_ssl_cert_dir);
++  free(Curl_ssl_cert_file);
++
+ #ifdef WIN32
+   Curl_win32_cleanup(init_flags);
+ #endif
+diff -ur curl-7.66.0.orig/lib/url.c curl-7.66.0/lib/url.c
+--- curl-7.66.0.orig/lib/url.c	2020-01-02 15:43:11.883921171 +0100
++++ curl-7.66.0/lib/url.c	2020-01-02 16:21:11.563880346 +0100
+@@ -524,6 +524,21 @@
+     if(result)
+       return result;
+ #endif
++    extern char * Curl_ssl_cert_dir;
++    extern char * Curl_ssl_cert_file;
++    if(Curl_ssl_cert_dir) {
++        if(result = Curl_setstropt(&set->str[STRING_SSL_CAPATH_ORIG], Curl_ssl_cert_dir))
++            return result;
++        if(result = Curl_setstropt(&set->str[STRING_SSL_CAPATH_PROXY], Curl_ssl_cert_dir))
++            return result;
++    }
++
++    if(Curl_ssl_cert_file) {
++        if(result = Curl_setstropt(&set->str[STRING_SSL_CAFILE_ORIG], Curl_ssl_cert_file))
++            return result;
++        if(result = Curl_setstropt(&set->str[STRING_SSL_CAFILE_PROXY], Curl_ssl_cert_file))
++            return result;
++    }
+   }
+ 
+   set->wildcard_enabled = FALSE;
-- 
2.24.1





Information forwarded to guix-patches <at> gnu.org:
bug#38873; Package guix-patches. (Sun, 12 Jan 2020 16:33:01 GMT) Full text and rfc822 format available.

Message #8 received at 38873 <at> debbugs.gnu.org (full text, mbox):

From: Jakub Kądziołka <kuba <at> kadziolka.net>
To: 38873 <at> debbugs.gnu.org
Subject: Patch submitted upstream
Date: Sun, 12 Jan 2020 17:32:47 +0100
For reference: I have submitted this patch to curl itself, it seems that
they find this unnecessary to have upstream:
https://github.com/curl/curl/pull/4809




Information forwarded to guix-patches <at> gnu.org:
bug#38873; Package guix-patches. (Mon, 13 Jan 2020 22:58:01 GMT) Full text and rfc822 format available.

Message #11 received at 38873 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Jakub Kądziołka <kuba <at> kadziolka.net>,
 38873 <at> debbugs.gnu.org
Subject: Re: [bug#38873] [PATCH] gnu: curl: Make libcurl respect SSL_CERT_{DIR, FILE}
Date: Mon, 13 Jan 2020 23:57:28 +0100
[Message part 1 (text/plain, inline)]
Jakub Kądziołka <kuba <at> kadziolka.net> writes:

> * gnu/packages/curl.scm (curl-7.66.0): Use patch.
> * gnu/packages/patches/libcurl-use-ssl-cert-env.patch: New file.
>
> This fixes the SSL errors occuring when trying to use rust:cargo's
> download functionality.
>
> As an additional advantage, this will probably allow removing some
> package-specific work-arounds that have already been made. I have
> found such work-arounds in cmake and kodi, but am not familiar enough
> with either to confidently remove them.

Thanks!  We should probably adjust the (native-search-paths ...) field
of cURL to account for these new variables too.  Can you also rebase it
on 'core-updates'?

From reading the upstream discussion, there does not seem to be any
inherent problems with the patch.  So, LGTM.  Are you willing to
maintain it when it inevitably requires porting to newer versions?  :-)
[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#38873; Package guix-patches. (Tue, 14 Jan 2020 17:00:02 GMT) Full text and rfc822 format available.

Message #14 received at 38873 <at> debbugs.gnu.org (full text, mbox):

From: Jakub Kądziołka <kuba <at> kadziolka.net>
To: 38873 <at> debbugs.gnu.org
Subject: [PATCH v2 core-updates] curl: Make libcurl respect SSL_CERT_DIR,
 SSL_CERT_FILE
Date: Tue, 14 Jan 2020 17:59:21 +0100
* gnu/packages/patches/libcurl-use-ssl-cert-env.patch: New file.
* gnu/packages/curl.scm (curl)[source]: Use the patch.
  [native-search-paths]: Add the new variables.
---
 gnu/packages/curl.scm                         | 20 ++++--
 .../patches/cmake-curl-certificates.patch     |  2 +
 .../kodi-set-libcurl-ssl-parameters.patch     |  2 +
 .../patches/libcurl-use-ssl-cert-env.patch    | 64 +++++++++++++++++++
 4 files changed, 84 insertions(+), 4 deletions(-)
 create mode 100644 gnu/packages/patches/libcurl-use-ssl-cert-env.patch

diff --git a/gnu/packages/curl.scm b/gnu/packages/curl.scm
index ee1cca449b..074ae32fb5 100644
--- a/gnu/packages/curl.scm
+++ b/gnu/packages/curl.scm
@@ -9,6 +9,7 @@
 ;;; Copyright © 2017, 2018 Tobias Geerinckx-Rice <me <at> tobias.gr>
 ;;; Copyright © 2018 Roel Janssen <roel <at> gnu.org>
 ;;; Copyright © 2019 Ricardo Wurmus <rekado <at> elephly.net>
+;;; Copyright © 2020 Jakub Kądziołka <kuba <at> kadziolka.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -57,7 +58,8 @@
                                 version ".tar.xz"))
             (sha256
              (base32
-              "0nh3j90w6b97wqcgxjfq55qhkz9s38955fbhwzv2fsi7483j895p"))))
+              "0nh3j90w6b97wqcgxjfq55qhkz9s38955fbhwzv2fsi7483j895p"))
+            (patches (search-patches "libcurl-use-ssl-cert-env.patch"))))
    (build-system gnu-build-system)
    (outputs '("out"
               "doc"))                             ;1.2 MiB of man3 pages
@@ -74,10 +76,20 @@
        ("pkg-config" ,pkg-config)
        ("python" ,python-wrapper)))
    (native-search-paths
-    ;; Note: This search path is respected by the `curl` command-line tool only.
-    ;; Ideally we would bake this into libcurl itself so other users can benefit,
-    ;; but it's not supported upstream due to thread safety concerns.
+    ;; Introduced by libcurl-use-ssl-cert-env.patch
     (list (search-path-specification
+           (variable "SSL_CERT_DIR")
+           (separator #f)                        ;single entry
+           (files '("etc/ssl/certs")))
+          (search-path-specification
+           (variable "SSL_CERT_FILE")
+           (file-type 'regular)
+           (separator #f)                        ;single entry
+           (files '("etc/ssl/certs/ca-certificates.crt")))
+    ;; Note: This search path is respected by the `curl` command-line tool only.
+    ;; Patching libcurl to read it too would bring no advantages and require
+    ;; maintaining a more complex patch.
+          (search-path-specification
            (variable "CURL_CA_BUNDLE")
            (file-type 'regular)
            (separator #f)                         ;single entry
diff --git a/gnu/packages/patches/cmake-curl-certificates.patch b/gnu/packages/patches/cmake-curl-certificates.patch
index 7fe2615271..e8cda5bd96 100644
--- a/gnu/packages/patches/cmake-curl-certificates.patch
+++ b/gnu/packages/patches/cmake-curl-certificates.patch
@@ -4,6 +4,8 @@ at all: <https://issues.guix.gnu.org/issue/37371>.
 This changes CMake such that commands honor SSL_CERT_FILE and SSL_CERT_DIR
 as well as /etc/ssl/certs.
 
+FIXME: This shouldn't be necessary anymore, see libcurl-use-ssl-cert-env.patch
+
 --- cmake-3.13.1/Source/cmCurl.cxx	2019-09-10 17:27:36.926907260 +0200
 +++ cmake-3.13.1/Source/cmCurl.cxx	2019-09-10 17:52:35.475903919 +0200
 @@ -2,11 +2,8 @@
diff --git a/gnu/packages/patches/kodi-set-libcurl-ssl-parameters.patch b/gnu/packages/patches/kodi-set-libcurl-ssl-parameters.patch
index 2f60737e30..98bff50712 100644
--- a/gnu/packages/patches/kodi-set-libcurl-ssl-parameters.patch
+++ b/gnu/packages/patches/kodi-set-libcurl-ssl-parameters.patch
@@ -1,6 +1,8 @@
 Kodi doesn't set the CAPATH and CAINFO parameters for libcurl. To make HTTPS
 connections work we can set them based on SSL_CERT_DIR and SSL_CERT_FILE.
 
+FIXME: This shouldn't be necessary anymore, see libcurl-use-ssl-cert-env.patch
+
 --- a/xbmc/filesystem/CurlFile.cpp
 +++ b/xbmc/filesystem/CurlFile.cpp
 @@ -626,5 +626,9 @@
diff --git a/gnu/packages/patches/libcurl-use-ssl-cert-env.patch b/gnu/packages/patches/libcurl-use-ssl-cert-env.patch
new file mode 100644
index 0000000000..c8e80b4445
--- /dev/null
+++ b/gnu/packages/patches/libcurl-use-ssl-cert-env.patch
@@ -0,0 +1,64 @@
+Make libcurl respect the SSL_CERT_{DIR,FILE} variables by default. The variables
+are fetched during initialization to preserve thread-safety (curl_global_init(3)
+must be called when no other threads exist).
+
+This fixes network functionality in rust:cargo, and probably removes the need
+for other future workarounds.
+===================================================================
+--- curl-7.66.0.orig/lib/easy.c	2020-01-02 15:43:11.883921171 +0100
++++ curl-7.66.0/lib/easy.c	2020-01-02 16:18:54.691882797 +0100
+@@ -134,6 +134,9 @@
+ #  pragma warning(default:4232) /* MSVC extension, dllimport identity */
+ #endif
+ 
++char * Curl_ssl_cert_dir = NULL;
++char * Curl_ssl_cert_file = NULL;
++
+ /**
+  * curl_global_init() globally initializes curl given a bitwise set of the
+  * different features of what to initialize.
+@@ -155,6 +158,9 @@
+ #endif
+   }
+ 
++  Curl_ssl_cert_dir = curl_getenv("SSL_CERT_DIR");
++  Curl_ssl_cert_file = curl_getenv("SSL_CERT_FILE");
++
+   if(!Curl_ssl_init()) {
+     DEBUGF(fprintf(stderr, "Error: Curl_ssl_init failed\n"));
+     return CURLE_FAILED_INIT;
+@@ -260,6 +266,9 @@
+   Curl_ssl_cleanup();
+   Curl_resolver_global_cleanup();
+ 
++  free(Curl_ssl_cert_dir);
++  free(Curl_ssl_cert_file);
++
+ #ifdef WIN32
+   Curl_win32_cleanup(init_flags);
+ #endif
+diff -ur curl-7.66.0.orig/lib/url.c curl-7.66.0/lib/url.c
+--- curl-7.66.0.orig/lib/url.c	2020-01-02 15:43:11.883921171 +0100
++++ curl-7.66.0/lib/url.c	2020-01-02 16:21:11.563880346 +0100
+@@ -524,6 +524,21 @@
+     if(result)
+       return result;
+ #endif
++    extern char * Curl_ssl_cert_dir;
++    extern char * Curl_ssl_cert_file;
++    if(Curl_ssl_cert_dir) {
++        if(result = Curl_setstropt(&set->str[STRING_SSL_CAPATH_ORIG], Curl_ssl_cert_dir))
++            return result;
++        if(result = Curl_setstropt(&set->str[STRING_SSL_CAPATH_PROXY], Curl_ssl_cert_dir))
++            return result;
++    }
++
++    if(Curl_ssl_cert_file) {
++        if(result = Curl_setstropt(&set->str[STRING_SSL_CAFILE_ORIG], Curl_ssl_cert_file))
++            return result;
++        if(result = Curl_setstropt(&set->str[STRING_SSL_CAFILE_PROXY], Curl_ssl_cert_file))
++            return result;
++    }
+   }
+ 
+   set->wildcard_enabled = FALSE;
-- 
2.24.1





Reply sent to Marius Bakke <mbakke <at> fastmail.com>:
You have taken responsibility. (Tue, 14 Jan 2020 23:38:02 GMT) Full text and rfc822 format available.

Notification sent to Jakub Kądziołka <kuba <at> kadziolka.net>:
bug acknowledged by developer. (Tue, 14 Jan 2020 23:38:02 GMT) Full text and rfc822 format available.

Message #19 received at 38873-done <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Jakub Kądziołka <kuba <at> kadziolka.net>,
 38873-done <at> debbugs.gnu.org
Subject: Re: [bug#38873] [PATCH v2 core-updates] curl: Make libcurl respect
 SSL_CERT_DIR, SSL_CERT_FILE
Date: Wed, 15 Jan 2020 00:37:20 +0100
[Message part 1 (text/plain, inline)]
Jakub Kądziołka <kuba <at> kadziolka.net> writes:

> * gnu/packages/patches/libcurl-use-ssl-cert-env.patch: New file.
> * gnu/packages/curl.scm (curl)[source]: Use the patch.
>   [native-search-paths]: Add the new variables.
> ---
>  gnu/packages/curl.scm                         | 20 ++++--
>  .../patches/cmake-curl-certificates.patch     |  2 +
>  .../kodi-set-libcurl-ssl-parameters.patch     |  2 +
>  .../patches/libcurl-use-ssl-cert-env.patch    | 64 +++++++++++++++++++
>  4 files changed, 84 insertions(+), 4 deletions(-)
>  create mode 100644 gnu/packages/patches/libcurl-use-ssl-cert-env.patch

The commit message forgot to mention the changed patches.  However I
opted to remove them, as the Kodi and CMake patches are harmless even
with your patch, but will encourage you to fix them regardless.  ;-)

Also added the new patch to gnu/local.mk and adjusted indentation of the
cURL comments.

Pushed as a76a343082d61d5303b61a9e4cbde4ab8515a1e7, thanks!
[signature.asc (application/pgp-signature, inline)]

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 12 Feb 2020 12:24:06 GMT) Full text and rfc822 format available.

This bug report was last modified 4 years and 47 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.