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.

Sign up for GitHub

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

Jump to bottom

Curl will clear the ex data of the X509_STORE in the ossl_connect_step2 with OpenSSL3.0 #11800

Closed
guoxinvmware opened this issue Sep 5, 2023 · 5 comments
Closed

Curl will clear the ex data of the X509_STORE in the ossl_connect_step2 with OpenSSL3.0 #11800

guoxinvmware opened this issue Sep 5, 2023 · 5 comments
Assignees
@icing
Labels
TLS

Comments

@guoxinvmware
Copy link

I did this

Steps:

  1. I used curl 8.2.1 to connect to a website with OpenSSL 3.0.
  2. I called the OpenSSL function X509_STORE_set_ex_data to set the external data with an X509_STORE.
  3. Then, I called the OpenSSL function SSL_CTX_set_verify with a callback function.
  4. In the callback function, I called the X509_STORE_get_ex_data to get the external data.

Expected:
4. To obtain the external data.

Actual:
4. the external data is NULL. Curl cleared it before the callback function.

Note:
This issue doesn't occur if I use curl with OpenSSL1.0.2.

My investigation:
Root cause:
In lib/vtls/openssl.c, line 280, the macro HAVE_SSL_X509_STORE_SHARE is only defined in curl when OpenSSL >= 1.1.0.
There is a function named Curl_ssl_setup_x509_store at line 3368, and it will be called in the ossl_connect_step2.
For OpenSSL3.0, it will call the OpenSSL function SSL_CTX_set_cert_store, if there is a cached_store and the cache_criteria_met is true.
The SSL_CTX_set_cert_store will clear the external data we set in the ssl_ctx before.

I expected the following

I expected that curl will not clear the ex data in the ossl_connect_step2 with OpenSSL3.0.

curl/libcurl version

curl 8.2.1

operating system

macOS 13.5.1 (22G90)
@bagder bagder added the TLS label Sep 5, 2023
@icing
Copy link
Contributor

icing commented Sep 5, 2023

I assume you use CURLOPT_SSL_CTX_FUNCTION to have a callback that installs your ex data at the store.

In curl v7.88.0 we chose to delay store initialization to a later point in time to have a fast handshake. This means that the store you see in your CURLOPT_SSL_CTX_FUNCTION is not the store that is later in effect. And your verify lookup no longer can find the ex data.

I think the best way to fix this is to load the "real" store in case an CURLOPT_SSL_CTX_FUNCTION is registered. The first handshake would take longer, but maybe you can live without that optimization.

@bagder wdyt?

@icing icing self-assigned this Sep 5, 2023
@bagder
Copy link
Member

bagder commented Sep 5, 2023

I'm trying to understand @guoxinvmware's use case. I don't think we (can) promise that you can pass data with OpenSSL's API like this.

I think the best way to fix this is to load the "real" store in case an CURLOPT_SSL_CTX_FUNCTION is registered

Agreed.

@guoxinvmware
Copy link
Author

guoxinvmware commented Sep 5, 2023 •
edited

Yes, you are right. I use it to install my ex data at the store.

I assume you use CURLOPT_SSL_CTX_FUNCTION to have a callback that installs your ex data at the store.

icing added a commit to icing/curl that referenced this issue Sep 6, 2023
@icing
When CURLOPT_SSL_CTX_FUNCTION is registered, init x509 store before
05574d9 
- refs curl#11800
- we delay loading the x509 store to shorten the handshake time.
  However an application callback installed via CURLOPT_SSL_CTX_FUNCTION
  may need to have the store loaded and try to manipulate it.
- load the x509 store before invoking the app callback
@icing icing mentioned this issue Sep 6, 2023
Closed
@icing
Copy link
Contributor

icing commented Sep 6, 2023

Just add a PR to make the x509 store fully initialised before an application callback is invoked.

Would be nice if you could verify that this fix works for you.

icing added a commit to icing/curl that referenced this issue Sep 7, 2023
@icing
When CURLOPT_SSL_CTX_FUNCTION is registered, init x509 store before
bbbabed 
- refs curl#11800
- we delay loading the x509 store to shorten the handshake time.
  However an application callback installed via CURLOPT_SSL_CTX_FUNCTION
  may need to have the store loaded and try to manipulate it.
- load the x509 store before invoking the app callback
icing added a commit to icing/curl that referenced this issue Sep 7, 2023
@icing
When CURLOPT_SSL_CTX_FUNCTION is registered, init x509 store before
abe193a 
- refs curl#11800
- we delay loading the x509 store to shorten the handshake time.
  However an application callback installed via CURLOPT_SSL_CTX_FUNCTION
  may need to have the store loaded and try to manipulate it.
- load the x509 store before invoking the app callback
@bagder bagder closed this as completed in c849062 Sep 7, 2023
@guoxinvmware
Copy link
Author

Verified with https://github.com/curl/curl/pull/11805/commits. This issue is fixed.
Thanks for your efforts!

ptitSeb pushed a commit to wasix-org/curl that referenced this issue Sep 25, 2023
@icing @ptitSeb
openssl: when CURLOPT_SSL_CTX_FUNCTION is registered, init x509 store…
dac061d 
… before

- we delay loading the x509 store to shorten the handshake time.
  However an application callback installed via CURLOPT_SSL_CTX_FUNCTION
  may need to have the store loaded and try to manipulate it.
- load the x509 store before invoking the app callback

Fixes curl#11800
Reported-by: guoxinvmware on github
Cloes curl#11805
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@icing icing

Labels
TLS
Milestone
No milestone
Development

No branches or pull requests
3 participants
@icing @bagder @guoxinvmware