Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

curl_global_cleanup/curl_global_init cycle leads to non-working SSL with libressl #12525

Closed
glandium opened this issue Dec 15, 2023 · 3 comments

Comments

@glandium
Copy link

I did this

Here the backstory: git-cinnabar uses curl via libgit's http.c, and because it avoids doing too many modifications to it, it ends up calling curl_global_init and curl_global_cleanup multiple times. This has always worked properly for git-cinnabar users... until macOS Sonoma, where things broke.

I'm not sure what older versions of macOS had, but it now uses curl 8.4.0 with libressl 3.3.6. It might have been using older versions of libressl before, or maybe openssl, I don't know. Anyways, what happens is that the call to EVP_cleanup() in ossl_cleanup() frees ciphers and digests lists, but the call to OPENSSL_init_ssl in ossl_init doesn't fill new cipher and digests lists, because they are statically initialized once, ever (their initialization uses pthread_once).

This causes SSL failures after calling curl_global_cleanup and curl_global_init.

This can be easily reproduced by taking docs/examples/https.c and apply this patch to it:

--- docs/examples/https.c
+++ docs/examples/https.c
@@ -34,6 +34,8 @@
   CURLcode res;
 
   curl_global_init(CURL_GLOBAL_DEFAULT);
+  curl_global_cleanup();
+  curl_global_init(CURL_GLOBAL_DEFAULT);
 
   curl = curl_easy_init();
   if(curl) {

This doesn't require macOS Sonoma to reproduce: this breaks similarly when running on a Linux host with curl 8.5.0 built against a manually built libressl 3.8.2.

The curl_global_cleanup manual says "You should call curl_global_cleanup once for each call you make to curl_global_init, after you are done using libcurl.", which implies the testcase is not doing anything illegal.

I expected the following

No response

curl/libcurl version

8.4.0

operating system

macOS Sonoma 14.2

glandium added a commit to glandium/git-cinnabar that referenced this issue Dec 15, 2023
glandium added a commit to glandium/git-cinnabar that referenced this issue Dec 15, 2023
@vszakats
Copy link
Member

vszakats commented Dec 15, 2023

Thanks for you report.

Possibly related: #11611

One theory is doing the same change for ossl_cleanup():

--- a/lib/vtls/openssl.c
+++ b/lib/vtls/openssl.c
@@ -1755,7 +1755,7 @@ static int ossl_init(void)
 static void ossl_cleanup(void)
 {
 #if (OPENSSL_VERSION_NUMBER >= 0x10100000L) &&  \
-  !defined(LIBRESSL_VERSION_NUMBER)
+  (!defined(LIBRESSL_VERSION_NUMBER) || LIBRESSL_VERSION_NUMBER >= 0x2070000fL)
   /* OpenSSL 1.1 deprecates all these cleanup functions and
      turns them into no-ops in OpenSSL 1.0 compatibility mode */
 #else

Or reverting #11611 if all else fails.

@glandium
Copy link
Author

Both options work.

vszakats added a commit to vszakats/curl that referenced this issue Dec 15, 2023
Earlier we bumped LibreSSL to use modern initialization, but did not
touch deinitialization due to uncertainties [1]. Fix it in this patch.

Regression from bec0c5b curl#11611

[1] curl#11611 (comment)

Reported-by: Mike Hommey
Fixes curl#12525
Closes #xxxxx
vszakats added a commit to vszakats/curl that referenced this issue Dec 15, 2023
Earlier we bumped LibreSSL to use modern initialization, but did not
touch deinitialization [1]. Fix it in this patch.

Regression from bec0c5b curl#11611

[1] curl#11611 (comment)

Reported-by: Mike Hommey
Fixes curl#12525
Closes curl#12526
glandium added a commit to glandium/git-cinnabar that referenced this issue Dec 24, 2023
glandium added a commit to glandium/git-cinnabar that referenced this issue Dec 24, 2023
glandium added a commit to glandium/git-cinnabar that referenced this issue Dec 24, 2023
glandium added a commit to glandium/git-cinnabar that referenced this issue Dec 24, 2023
@hjmallon
Copy link
Contributor

hjmallon commented Jan 2, 2024

As mentioned by @Bo98 in hombrew repo this regression has got into macOS libcurl some time around macOS 14.2. I have filed a feedback assistant with Apple.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants