git, openssl, curl: Respect $NIX_SSL_CERT_FILE #24121
Conversation
Slightly modified version of 942dbf8
|
@domenkozar, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edef1c, @ikervagyok and @edolstra to be potential reviewers. |
| + if (getenv("NIX_SSL_CERT_FILE")) | ||
| + set_from_env(&ssl_cainfo, "NIX_SSL_CERT_FILE"); | ||
| + else | ||
| + set_from_env(&ssl_cainfo, "SSL_CERT_FILE"); |
There was a problem hiding this comment.
This should not appear here. Git does follow SSL_CERT_FILE by default. There is no reason to keep this line if we use NIX_SSL_CERT_FILE. Git uses openssl and without this override, the underlying openssl will look at SSL_CERT_FILE anyway.
tl;dr set_from_env(&ssl_cainfo, "SSL_CERT_FILE"); should not appear here.
There was a problem hiding this comment.
If so, NIX_SSL_CERT_FILE is tried by the patched openssl as well...
There was a problem hiding this comment.
Anyway, NIX_SSL_CERT_FILE should be a default value, not a global override. If someone knows better than us where to look for the CA bundle, let them have it their way.
There was a problem hiding this comment.
In this case, SSL_CERT_FILE was introduced by #14617. There was not much investigation to understand the issue. Maybe it was due to some misconfigured openssl, Or this i really needed. Who knows... Anyway, it's NIX_SSL_CERT_FILE which is needed, not "SSL_CERT_FILE".
| - env = curlx_getenv("SSL_CERT_FILE"); | ||
| + env = curlx_getenv("NIX_SSL_CERT_FILE"); | ||
| + if(!env) | ||
| + env = curlx_getenv("SSL_CERT_FILE"); |
There was a problem hiding this comment.
Same as above, let SSL_CERT_FILE return to its rank of default value if none provided. We now use "NIX_SSL_CERT_FILE" as first priority override.
There was a problem hiding this comment.
I think this is intentional because of Darwin #23605.
There was a problem hiding this comment.
Sorry, this one is correct because it is how curl works...
Fixes #23605
cc @edolstra @vcunat