Re: [PATCH v2 1/3] http: add support for selecting SSL backends at runtime

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH v2 1/3] http: add support for selecting SSL backends at runtime
Date: Thu, 13 Dec 2018 10:33:41 +0100	[thread overview]
Message-ID: <87sgz1n53e.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <85bd0fb27fcf7615b3f927344fd77ea49b9f5dcb.1540493630.git.gitgitgadget@gmail.com>

On Thu, Oct 25 2018, Johannes Schindelin via GitGitGadget wrote:

> 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?