[WIP] libcurl: add new API function curl_global_sslget #2063
Conversation
We specifically disallow this (at least for the time being). You have to settle on one backend, and that's that. Not that we take pains in ensuring that that is thread-safe: if two threads happen to try to set the SSL backend at the very same time, to different values, then they might both "succeed", as there is some nanosecond or three between the check and the setting (this goes by the acronym TOCTOU). However, I would hope that we do not slam the door shut to maybe eventually use different SSL backends in parallel, as they each have their pros and cons. |
| CURLSSLBACKEND_SCHANNEL = 8, | ||
| CURLSSLBACKEND_DARWINSSL = 9, | ||
| CURLSSLBACKEND_AXTLS = 10, | ||
| CURLSSLBACKEND_MBEDTLS = 11 |
There was a problem hiding this comment.
I am not sure that we should list the numerical values here, a mere list of the identifiers should suffice, no?
include/curl/curl.h
Outdated
| @@ -2378,6 +2378,9 @@ typedef enum { | |||
| CURL_EXTERN CURLsslset curl_global_sslset(curl_sslbackend id, const char *name, | |||
| const curl_ssl_backend ***avail); | |||
|
|
|||
| /* Get the selected SSL backend. */ | |||
| CURL_EXTERN const curl_ssl_backend *curl_global_sslget(); | |||
There was a problem hiding this comment.
GCC and Clang complain because there is no void between those parentheses. I am fairly certain that things will work nicely if this line is changed to
CURL_EXTERN const curl_ssl_backend *curl_global_sslget(void);
|
I reviewed this relatively quickly (it is not a big change, so it was easy to fit in)... But I wonder whether Of course, there is one crucial difference: |
curl_global_sslget retrieves the selected SSL backend id/name. work in progress. Ref: curl#2063
jay
force-pushed
the
curl_global_sslget
branch
from
125b9cc
to
7026d60
Compare
|
Thanks for the review. Curl_ssl_backend is not public API and also I didn't like that it set the backend so that's why i'm not interested in it.
This is what concerns me. The documentation for sslset basically states it can only be used as a one-off but based on some of the conversations I read at the time it seemed like maybe you guys wanted to expand that at some later point. The use case btw is there is CURLOPT_CAINFO support coming for schannel likely during the next feature window but I would like to make it so that the tool doesn't set CURLOPT_CAINFO unless explicitly specified by the user in the case of schannel. Right now on Windows no matter the SSL backend it automatically looks for some ca certs and with the coming changes those certs will now be used instead of the default certificate store in the case of WinSSL. Changing that behavior using today's available info would look something like this I think: |
|
an alternative would be to put this info into the struct |
True. Sorry for suggesting this ;-) |
|
Closing due to inactivity. We can re-open again if there's a renewed effort. |
I wanted an easy way to tell the SSL backend in use without a curl easy handle. CURLINFO_TLS_SSL_PTR can do it but only with an easy handle. I wanted to be able to tell earlier in the process. curl_version_info sslversion string gives an idea:
I can parse that of course but I thought it would be easier if I could do this:
if(curl_global_sslget()->id == CURLSSLBACKEND_SCHANNEL)
I'm unclear on the thread safety, specifically whether we can guarantee that once the backend id != CURLSSLBACKEND_NONE that it's not going to change, because if it is then this call is not thread safe which I'd imagine would make it a lot less desirable for users.
This is a work in progress and I'm not totally committed to it. I'm looking at a situation in the tool where we might need the backend information before easy handle creation and I didn't want to parse sslversion.
/cc @dscho