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

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

[Jakub Kądziołka]

* 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@tobias.gr>
 ;;; Copyright © 2018 Roel Janssen <roel@gnu.org>
 ;;; Copyright © 2019 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2020 Jakub Kądziołka <kuba@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

[bug#38873] Patch submitted upstream

[Jakub Kądziołka]

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

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

[Marius Bakke]

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

Jakub Kądziołka <kuba@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?  :-)

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

[bug#38873] [PATCH v2 core-updates] curl: Make libcurl respect SSL_CERT_DIR, SSL_CERT_FILE

[Jakub Kądziołka]

* 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@tobias.gr>
 ;;; Copyright © 2018 Roel Janssen <roel@gnu.org>
 ;;; Copyright © 2019 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2020 Jakub Kądziołka <kuba@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

bug#38873: [PATCH v2 core-updates] curl: Make libcurl respect SSL_CERT_DIR, SSL_CERT_FILE

[Marius Bakke]

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

Jakub Kądziołka <kuba@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!

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