Re: [PATCH v2 1/3] http: add support for selecting SSL backends at runtime
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Dec 13, 2018, 09:33 UTC
- Message-ID
- <87sgz1n53e.fsf@evledraar.gmail.com>
- In-Reply-To
- <85bd0fb27fcf7615b3f927344fd77ea49b9f5dcb.1540493630.git.gitgitgadget@gmail.com>
On Thu, Oct 25 2018, Johannes Schindelin via GitGitGadget wrote:
Show 100 quoted lines
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> As of version 7.56.0, curl supports being compiled with multiple SSL
> backends.
>
> This patch adds the Git side of that feature: by setting http.sslBackend
> to "openssl" or "schannel", Git for Windows can now choose the SSL
> backend at runtime.
>
> This comes in handy on Windows because Secure Channel ("schannel") is
> the native solution, accessing the Windows Credential Store, thereby
> allowing for enterprise-wide management of certificates. For historical
> reasons, Git for Windows needs to support OpenSSL still, as it has
> previously been the only supported SSL backend in Git for Windows for
> almost a decade.
>
> The patch has been carried in Git for Windows for over a year, and is
> considered mature.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> Documentation/config.txt | 5 +++++
> http.c | 35 +++++++++++++++++++++++++++++++++++
> 2 files changed, 40 insertions(+)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 154683321..7d38f0bf1 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1984,6 +1984,11 @@ http.sslCAPath::
> with when fetching or pushing over HTTPS. Can be overridden
> by the `GIT_SSL_CAPATH` environment variable.
>
> +http.sslBackend::
> + Name of the SSL backend to use (e.g. "openssl" or "schannel").
> + This option is ignored if cURL lacks support for choosing the SSL
> + backend at runtime.
> +
> http.pinnedpubkey::
> Public key of the https service. It may either be the filename of
> a PEM or DER encoded public key file or a string starting with
> diff --git a/http.c b/http.c
> index 98ff12258..7fb37a061 100644
> --- a/http.c
> +++ b/http.c
> @@ -155,6 +155,8 @@ static struct active_request_slot *active_queue_head;
>
> static char *cached_accept_language;
>
> +static char *http_ssl_backend;
> +
> size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
> {
> size_t size = eltsize * nmemb;
> @@ -302,6 +304,12 @@ static int http_options(const char *var, const char *value, void *cb)
> curl_ssl_try = git_config_bool(var, value);
> return 0;
> }
> + if (!strcmp("http.sslbackend", var)) {
> + free(http_ssl_backend);
> + http_ssl_backend = xstrdup_or_null(value);
> + return 0;
> + }
> +
> if (!strcmp("http.minsessions", var)) {
> min_curl_sessions = git_config_int(var, value);
> #ifndef USE_CURL_MULTI
> @@ -995,6 +1003,33 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)
> git_config(urlmatch_config_entry, &config);
> free(normalized_url);
>
> +#if LIBCURL_VERSION_NUM >= 0x073800
> + if (http_ssl_backend) {
> + const curl_ssl_backend **backends;
> + struct strbuf buf = STRBUF_INIT;
> + int i;
> +
> + switch (curl_global_sslset(-1, http_ssl_backend, &backends)) {
> + case CURLSSLSET_UNKNOWN_BACKEND:
> + strbuf_addf(&buf, _("Unsupported SSL backend '%s'. "
> + "Supported SSL backends:"),
> + http_ssl_backend);
> + for (i = 0; backends[i]; i++)
> + strbuf_addf(&buf, "\n\t%s", backends[i]->name);
> + die("%s", buf.buf);
> + case CURLSSLSET_NO_BACKENDS:
> + die(_("Could not set SSL backend to '%s': "
> + "cURL was built without SSL backends"),
> + http_ssl_backend);
> + case CURLSSLSET_TOO_LATE:
> + die(_("Could not set SSL backend to '%s': already set"),
> + http_ssl_backend);
> + case CURLSSLSET_OK:
> + break; /* Okay! */
> + }
> + }
> +#endif
> +
> if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)
> die("curl_global_init failed");Here's someone who upgraded to 2.20 on Arch linux & started getting "Could not set..." errors because of this change: https://www.reddit.com/r/git/comments/a5ne5v/git_fatal_could_not_set_ssl_backend_to_openssl/
I don't know the context well enough, but is there perhaps enough info here so we could give a better error message, e.g. "don't set xyz twice in your config", or just emit a warning?