]> git.ipfire.org Git - thirdparty/curl.git/commitdiff
openssl: save and restore OpenSSL error queue in two functions
authorViktor Szakats <commit@vsz.me>
Fri, 8 Aug 2025 22:45:34 +0000 (00:45 +0200)
committerViktor Szakats <commit@vsz.me>
Wed, 13 Aug 2025 16:54:08 +0000 (18:54 +0200)
After merging #18228, I reviewed whether the clearing of the error queue
may interfere with preceding code. Turns out there may be a preceding
`SSL_Connect()` call.

This patch replaces the previous fix of clearing the error queue with
saving and restoring it in two functions which may be called between
the connect call and the `SSL_get_error()` call following it:
- `ossl_log_tls12_secret()`
- `Curl_ssl_setup_x509_store()`

The `ERR_set_mark()`, `ERR_pop_to_mark()` functions are present in all
supported OpenSSL and LibreSSL versions. Also in BoringSSL since its
initial commit.

OpenSSL may modify its error queue in all API calls that can fail.

Thanks-to: Viktor Dukhovni
Ref: https://github.com/curl/curl/issues/18190#issuecomment-3167702142
Ref: https://github.com/curl/curl/issues/18190#issuecomment-3169211739
Ref: https://github.com/curl/curl/issues/18190#issuecomment-3169988050

Follow-up to 8ec241bc990bc88c4f4f7275d81f9fb75b562a7a #18228 #18190
Ref: e8b00fcd6a0c7ff179cebb3615ccebf1f6790b69 #10432 #10389
Fixes #18190
Closes #18234

lib/vtls/openssl.c

index dc4a6d122cee9688a9c7f0888a1eb9df00d44ad3..c4c2981108c27fd3355ff17e5e91884f58dcac8d 100644 (file)
@@ -850,13 +850,19 @@ static void ossl_keylog_callback(const SSL *ssl, const char *line)
 static void
 ossl_log_tls12_secret(const SSL *ssl, bool *keylog_done)
 {
-  const SSL_SESSION *session = SSL_get_session(ssl);
+  const SSL_SESSION *session;
   unsigned char client_random[SSL3_RANDOM_SIZE];
   unsigned char master_key[SSL_MAX_MASTER_KEY_LENGTH];
   int master_key_length = 0;
 
-  if(!session || *keylog_done)
+  ERR_set_mark();
+
+  session = SSL_get_session(ssl);
+
+  if(!session || *keylog_done) {
+    ERR_pop_to_mark();
     return;
+  }
 
 #if OPENSSL_VERSION_NUMBER >= 0x10100000L
   /* ssl->s3 is not checked in OpenSSL 1.1.0-pre6, but let's assume that
@@ -872,6 +878,8 @@ ossl_log_tls12_secret(const SSL *ssl, bool *keylog_done)
   }
 #endif
 
+  ERR_pop_to_mark();
+
   /* The handshake has not progressed sufficiently yet, or this is a TLS 1.3
    * session (when curl was built with older OpenSSL headers and running with
    * newer OpenSSL runtime libraries). */
@@ -3327,10 +3335,8 @@ static CURLcode import_windows_cert_store(struct Curl_easy *data,
         continue;
 
       x509 = d2i_X509(NULL, &encoded_cert, (long)pContext->cbCertEncoded);
-      if(!x509) {
-        ERR_clear_error();
+      if(!x509)
         continue;
-      }
 
       /* Try to import the certificate. This may fail for legitimate
          reasons such as duplicate certificate, which is allowed by MS but
@@ -3661,6 +3667,8 @@ CURLcode Curl_ssl_setup_x509_store(struct Curl_cfilter *cf,
     !ssl_config->primary.CRLfile &&
     !ssl_config->native_ca_store;
 
+  ERR_set_mark();
+
   cached_store = ossl_get_cached_x509_store(cf, data);
   if(cached_store && cache_criteria_met && X509_STORE_up_ref(cached_store)) {
     SSL_CTX_set_cert_store(ssl_ctx, cached_store);
@@ -3674,6 +3682,8 @@ CURLcode Curl_ssl_setup_x509_store(struct Curl_cfilter *cf,
     }
   }
 
+  ERR_pop_to_mark();
+
   return result;
 }
 #else /* HAVE_SSL_X509_STORE_SHARE */
@@ -3681,9 +3691,17 @@ CURLcode Curl_ssl_setup_x509_store(struct Curl_cfilter *cf,
                                    struct Curl_easy *data,
                                    SSL_CTX *ssl_ctx)
 {
-  X509_STORE *store = SSL_CTX_get_cert_store(ssl_ctx);
+  CURLcode result;
+  X509_STORE *store;
+
+  ERR_set_mark();
 
-  return ossl_populate_x509_store(cf, data, store);
+  store = SSL_CTX_get_cert_store(ssl_ctx);
+  result = ossl_populate_x509_store(cf, data, store);
+
+  ERR_pop_to_mark();
+
+  return result;
 }
 #endif /* HAVE_SSL_X509_STORE_SHARE */