{"thread":{"id":"49568","subject":"[PATCH 0/3] Allow choosing the SSL backend cURL uses (plus related patches)","startedAt":"2018-10-15T10:14:45Z","lastAt":"2018-12-13T13:16:12Z","messageCount":28,"participants":["Johannes Schindelin via GitGitGadget","Brendan Forster via GitGitGadget","Eric Sunshine","Junio C Hamano","Jeff King","Johannes Schindelin","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"360506","messageId":"pull.46.git.gitgitgadget@gmail.com","threadId":"49568","inReplyTo":null,"subject":"[PATCH 0/3] Allow choosing the SSL backend cURL uses (plus related patches)","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-10-15T10:14:42Z","receivedAt":"2018-10-15T10:14:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This topic branch brings support for choosing cURL's SSL backend (a feature\ndeveloped in Git for Windows' context) at runtime via http.sslBackend, and\ntwo more patches that are related (and only of interest for Windows users).\n\nBrendan Forster (1):\n http: add support for disabling SSL revocation checks in cURL\n\nJohannes Schindelin (2):\n http: add support for selecting SSL backends at runtime\n http: when using Secure Channel, ignore sslCAInfo by default\n\n Documentation/config.txt | 21 ++++++++++++\n http.c | 71 +++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 91 insertions(+), 1 deletion(-)\n\n\nbase-commit: 5a0cc8aca797dbd7d2be3b67458ff880ed45cddf\nPublished-As: https://github.com/gitgitgadget/git/releases/tags/pr-46%2Fdscho%2Fhttp-ssl-backend-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-46/dscho/http-ssl-backend-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/46\n-- \ngitgitgadget\n"},{"id":"360507","messageId":"8c5ecdb6c9a73405036672db8bc1e36c5c6c6951.1539598481.git.gitgitgadget@gmail.com","threadId":"49568","inReplyTo":"pull.46.git.gitgitgadget@gmail.com","subject":"[PATCH 1/3] http: add support for selecting SSL backends at runtime","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-10-15T10:14:43Z","receivedAt":"2018-10-15T10:14:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nAs of version 7.56.0, curl supports being compiled with multiple SSL\nbackends.\n\nThis patch adds the Git side of that feature: by setting http.sslBackend\nto \"openssl\" or \"schannel\", Git for Windows can now choose the SSL\nbackend at runtime.\n\nThis comes in handy on Windows because Secure Channel (\"schannel\") is\nthe native solution, accessing the Windows Credential Store, thereby\nallowing for enterprise-wide management of certificates. For historical\nreasons, Git for Windows needs to support OpenSSL still, as it has\npreviously been the only supported SSL backend in Git for Windows for\nalmost a decade.\n\nThe patch has been carried in Git for Windows for over a year, and is\nconsidered mature.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/config.txt | 5 +++++\n http.c | 35 +++++++++++++++++++++++++++++++++++\n 2 files changed, 40 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1546833213..7d38f0bf1a 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1984,6 +1984,11 @@ http.sslCAPath::\n \twith when fetching or pushing over HTTPS. Can be overridden\n \tby the `GIT_SSL_CAPATH` environment variable.\n \n+http.sslBackend::\n+\tName of the SSL backend to use (e.g. \"openssl\" or \"schannel\").\n+\tThis option is ignored if cURL lacks support for choosing the SSL\n+\tbackend at runtime.\n+\n http.pinnedpubkey::\n \tPublic key of the https service. It may either be the filename of\n \ta PEM or DER encoded public key file or a string starting with\ndiff --git a/http.c b/http.c\nindex 98ff122585..7fb37a061b 100644\n--- a/http.c\n+++ b/http.c\n@@ -155,6 +155,8 @@ static struct active_request_slot *active_queue_head;\n \n static char *cached_accept_language;\n \n+static char *http_ssl_backend;\n+\n size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)\n {\n \tsize_t size = eltsize * nmemb;\n@@ -302,6 +304,12 @@ static int http_options(const char *var, const char *value, void *cb)\n \t\tcurl_ssl_try = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(\"http.sslbackend\", var)) {\n+\t\tfree(http_ssl_backend);\n+\t\thttp_ssl_backend = xstrdup_or_null(value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(\"http.minsessions\", var)) {\n \t\tmin_curl_sessions = git_config_int(var, value);\n #ifndef USE_CURL_MULTI\n@@ -995,6 +1003,33 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)\n \tgit_config(urlmatch_config_entry, &config);\n \tfree(normalized_url);\n \n+#if LIBCURL_VERSION_NUM >= 0x073800\n+\tif (http_ssl_backend) {\n+\t\tconst curl_ssl_backend **backends;\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tint i;\n+\n+\t\tswitch (curl_global_sslset(-1, http_ssl_backend, &backends)) {\n+\t\tcase CURLSSLSET_UNKNOWN_BACKEND:\n+\t\t\tstrbuf_addf(&buf, _(\"Unsupported SSL backend '%s'. \"\n+\t\t\t\t\t \"Supported SSL backends:\"),\n+\t\t\t\t\t http_ssl_backend);\n+\t\t\tfor (i = 0; backends[i]; i++)\n+\t\t\t\tstrbuf_addf(&buf, \"\\n\\t%s\", backends[i]->name);\n+\t\t\tdie(\"%s\", buf.buf);\n+\t\tcase CURLSSLSET_NO_BACKENDS:\n+\t\t\tdie(_(\"Could not set SSL backend to '%s': \"\n+\t\t\t \"cURL was built without SSL backends\"),\n+\t\t\t http_ssl_backend);\n+\t\tcase CURLSSLSET_TOO_LATE:\n+\t\t\tdie(_(\"Could not set SSL backend to '%s': already set\"),\n+\t\t\t http_ssl_backend);\n+\t\tcase CURLSSLSET_OK:\n+\t\t\tbreak; /* Okay! */\n+\t\t}\n+\t}\n+#endif\n+\n \tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n \t\tdie(\"curl_global_init failed\");\n \n-- \ngitgitgadget\n\n"},{"id":"360508","messageId":"764791d13d20478639402e7af95e901223444240.1539598481.git.gitgitgadget@gmail.com","threadId":"49568","inReplyTo":"pull.46.git.gitgitgadget@gmail.com","subject":"[PATCH 2/3] http: add support for disabling SSL revocation checks in cURL","fromName":"Brendan Forster via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-10-15T10:14:45Z","receivedAt":"2018-10-15T10:14:48Z","isPatch":true,"sender":{"key":"name:Brendan Forster","avatar":null},"body":"From: Brendan Forster \n\nThis adds support for a new http.schannelCheckRevoke config value.\n\nThis config value is only used if http.sslBackend is set to \"schannel\",\nwhich forces cURL to use the Windows Certificate Store when validating\nserver certificates associated with a remote server.\n\nThis config value should only be set to \"false\" if you are in an\nenvironment where revocation checks are blocked by the network, with\nno alternative options.\n\nThis is only supported in cURL 7.44 or later.\n\nNote: an earlier iteration tried to use the config setting\nhttp.schannel.checkRevoke, but the http.* config settings can be limited\nto specific URLs via http..* (which would mistake `schannel` for a\nURL).\n\nHelped by Agustín Martín Barbero.\n\nSigned-off-by: Brendan Forster \nSigned-off-by: Johannes Schindelin \n---\n Documentation/config.txt | 8 ++++++++\n http.c | 17 +++++++++++++++++\n 2 files changed, 25 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 7d38f0bf1a..d569ebd49e 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1989,6 +1989,14 @@ http.sslBackend::\n \tThis option is ignored if cURL lacks support for choosing the SSL\n \tbackend at runtime.\n \n+http.schannelCheckRevoke::\n+\tUsed to enforce or disable certificate revocation checks in cURL\n+\twhen http.sslBackend is set to \"schannel\". Defaults to `true` if\n+\tunset. Only necessary to disable this if Git consistently errors\n+\tand the message is about checking the revocation status of a\n+\tcertificate. This option is ignored if cURL lacks support for\n+\tsetting the relevant SSL option at runtime.\n+\n http.pinnedpubkey::\n \tPublic key of the https service. It may either be the filename of\n \ta PEM or DER encoded public key file or a string starting with\ndiff --git a/http.c b/http.c\nindex 7fb37a061b..8998056b60 100644\n--- a/http.c\n+++ b/http.c\n@@ -157,6 +157,8 @@ static char *cached_accept_language;\n \n static char *http_ssl_backend;\n \n+static int http_schannel_check_revoke = 1;\n+\n size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)\n {\n \tsize_t size = eltsize * nmemb;\n@@ -310,6 +312,11 @@ static int http_options(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(\"http.schannelcheckrevoke\", var)) {\n+\t\thttp_schannel_check_revoke = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(\"http.minsessions\", var)) {\n \t\tmin_curl_sessions = git_config_int(var, value);\n #ifndef USE_CURL_MULTI\n@@ -811,6 +818,16 @@ static CURL *get_curl_handle(void)\n \t}\n #endif\n \n+\tif (http_ssl_backend && !strcmp(\"schannel\", http_ssl_backend) &&\n+\t !http_schannel_check_revoke) {\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+\t\tcurl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);\n+#else\n+\t\twarning(\"CURLSSLOPT_NO_REVOKE not applied to curl SSL options because\\n\"\n+\t\t\t\"your curl version is too old (>= 7.44.0)\");\n+#endif\n+\t}\n+\n \tif (http_proactive_auth)\n \t\tinit_curl_http_auth(result);\n \n-- \ngitgitgadget\n\n"},{"id":"360509","messageId":"9927e4ce628df99f7a8e88dba1df54244e2fa158.1539598481.git.gitgitgadget@gmail.com","threadId":"49568","inReplyTo":"pull.46.git.gitgitgadget@gmail.com","subject":"[PATCH 3/3] http: when using Secure Channel, ignore sslCAInfo by default","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-10-15T10:14:46Z","receivedAt":"2018-10-15T10:14:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nAs of cURL v7.60.0, the Secure Channel backend can use the certificate\nbundle provided via `http.sslCAInfo`, but that would override the\nWindows Certificate Store. Since this is not desirable by default, let's\ntell Git to not ask cURL to use that bundle by default when the `schannel`\nbackend was configured via `http.sslBackend`, unless\n`http.schannelUseSSLCAInfo` overrides this behavior.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/config.txt | 8 ++++++++\n http.c | 19 ++++++++++++++++++-\n 2 files changed, 26 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex d569ebd49e..1f6a6a4a6f 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1997,6 +1997,14 @@ http.schannelCheckRevoke::\n \tcertificate. This option is ignored if cURL lacks support for\n \tsetting the relevant SSL option at runtime.\n \n+http.schannelUseSSLCAInfo::\n+\tAs of cURL v7.60.0, the Secure Channel backend can use the\n+\tcertificate bundle provided via `http.sslCAInfo`, but that would\n+\toverride the Windows Certificate Store. Since this is not desirable\n+\tby default, Git will tell cURL not to use that bundle by default\n+\twhen the `schannel` backend was configured via `http.sslBackend`,\n+\tunless `http.schannelUseSSLCAInfo` overrides this behavior.\n+\n http.pinnedpubkey::\n \tPublic key of the https service. It may either be the filename of\n \ta PEM or DER encoded public key file or a string starting with\ndiff --git a/http.c b/http.c\nindex 8998056b60..a0a8b93785 100644\n--- a/http.c\n+++ b/http.c\n@@ -158,6 +158,12 @@ static char *cached_accept_language;\n static char *http_ssl_backend;\n \n static int http_schannel_check_revoke = 1;\n+/*\n+ * With the backend being set to `schannel`, setting sslCAinfo would override\n+ * the Certificate Store in cURL v7.60.0 and later, which is not what we want\n+ * by default.\n+ */\n+static int http_schannel_use_ssl_cainfo;\n \n size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)\n {\n@@ -317,6 +323,11 @@ static int http_options(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(\"http.schannelusesslcainfo\", var)) {\n+\t\thttp_schannel_use_ssl_cainfo = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(\"http.minsessions\", var)) {\n \t\tmin_curl_sessions = git_config_int(var, value);\n #ifndef USE_CURL_MULTI\n@@ -869,7 +880,13 @@ static CURL *get_curl_handle(void)\n \tif (ssl_pinnedkey != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_PINNEDPUBLICKEY, ssl_pinnedkey);\n #endif\n-\tif (ssl_cainfo != NULL)\n+\tif (http_ssl_backend && !strcmp(\"schannel\", http_ssl_backend) &&\n+\t !http_schannel_use_ssl_cainfo) {\n+\t\tcurl_easy_setopt(result, CURLOPT_CAINFO, NULL);\n+#if LIBCURL_VERSION_NUM >= 0x073400\n+\t\tcurl_easy_setopt(result, CURLOPT_PROXY_CAINFO, NULL);\n+#endif\n+\t} else if (ssl_cainfo != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);\n \n \tif (curl_low_speed_limit > 0 && curl_low_speed_time > 0) {\n-- \ngitgitgadget\n"},{"id":"360521","messageId":"CAPig+cT2K9hCTaLbTW-3ueKV6vaokkLiVSUvZ7yZ+sncZnjvNw@mail.gmail.com","threadId":"49568","inReplyTo":"8c5ecdb6c9a73405036672db8bc1e36c5c6c6951.1539598481.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] http: add support for selecting SSL backends at runtime","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-10-15T14:06:14Z","receivedAt":"2018-10-15T14:06:28Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Oct 15, 2018 at 6:14 AM Johannes Schindelin via GitGitGadget\n wrote:\n> This patch adds the Git side of that feature: by setting http.sslBackend\n> to \"openssl\" or \"schannel\", Git for Windows can now choose the SSL\n> backend at runtime.\n> [...]\n> Signed-off-by: Johannes Schindelin \n> ---\n> diff --git a/http.c b/http.c\n> @@ -995,6 +1003,33 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)\n> +#if LIBCURL_VERSION_NUM >= 0x073800\n> + if (http_ssl_backend) {\n> + const curl_ssl_backend **backends;\n> + struct strbuf buf = STRBUF_INIT;\n> + int i;\n> +\n> + switch (curl_global_sslset(-1, http_ssl_backend, &backends)) {\n> + case CURLSSLSET_UNKNOWN_BACKEND:\n> + strbuf_addf(&buf, _(\"Unsupported SSL backend '%s'. \"\n> + \"Supported SSL backends:\"),\n> + http_ssl_backend);\n> + for (i = 0; backends[i]; i++)\n> + strbuf_addf(&buf, \"\\n\\t%s\", backends[i]->name);\n> + die(\"%s\", buf.buf);\n\nThis is the only 'case' arm which utilizes 'strbuf buf', and it\ndie()s, so no leak despite a lack of strbuf_release(). Okay.\n\n> + case CURLSSLSET_NO_BACKENDS:\n> + die(_(\"Could not set SSL backend to '%s': \"\n> + \"cURL was built without SSL backends\"),\n> + http_ssl_backend);\n> + case CURLSSLSET_TOO_LATE:\n> + die(_(\"Could not set SSL backend to '%s': already set\"),\n> + http_ssl_backend);\n> + case CURLSSLSET_OK:\n> + break; /* Okay! */\n> + }\n> + }\n> +#endif\n"},{"id":"360522","messageId":"CAPig+cQFb3S0Lm+huUZDN4aw9rWwinh0iZp12ss1zVKpJ=2MdA@mail.gmail.com","threadId":"49568","inReplyTo":"764791d13d20478639402e7af95e901223444240.1539598481.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] http: add support for disabling SSL revocation checks in cURL","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-10-15T14:10:32Z","receivedAt":"2018-10-15T14:10:45Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Oct 15, 2018 at 6:14 AM Brendan Forster via GitGitGadget\n wrote:\n> This config value is only used if http.sslBackend is set to \"schannel\",\n> which forces cURL to use the Windows Certificate Store when validating\n> server certificates associated with a remote server.\n>\n> This is only supported in cURL 7.44 or later.\n> [...]\n> Signed-off-by: Brendan Forster \n> ---\n> diff --git a/http.c b/http.c\n> @@ -811,6 +818,16 @@ static CURL *get_curl_handle(void)\n> + if (http_ssl_backend && !strcmp(\"schannel\", http_ssl_backend) &&\n> + !http_schannel_check_revoke) {\n> +#if LIBCURL_VERSION_NUM >= 0x072c00\n> + curl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);\n> +#else\n> + warning(\"CURLSSLOPT_NO_REVOKE not applied to curl SSL options because\\n\"\n> + \"your curl version is too old (>= 7.44.0)\");\n\nThis message is confusing. If your curl is too old, shouldn't the \">=\" be a \"<\"?\n\n> +#endif\n> + }\n"},{"id":"360575","messageId":"xmqq1s8q34g2.fsf@gitster-ct.c.googlers.com","threadId":"49568","inReplyTo":"764791d13d20478639402e7af95e901223444240.1539598481.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] http: add support for disabling SSL revocation checks in cURL","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-16T04:23:25Z","receivedAt":"2018-10-16T04:23:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Brendan Forster via GitGitGadget\" writes:\n\n> Note: an earlier iteration tried to use the config setting\n> http.schannel.checkRevoke, but the http.* config settings can be limited\n> to specific URLs via http..* (which would mistake `schannel` for a\n> URL).\n\nYeah, \"http.schannel.anything\" would not work, but is this note\nrelevant here? As far as the git development community as a whole\nis concerned, this is the first iteration of the patch we see and\nreview.\n\nIn any case, you can use \"http..$variable\" to say \"I want the\nhttp.$variable to be in effect but only when I am talking to \".\nDoes it make sense for this new variable, too? That is, does it\nbenefit the users to be able to do something like this?\n\n [http] schannelCheckRevoke = no\n [http \"https://microsoft.com/\"] schannelCheckRevoke = yes\n\nI am guessing that the answer is yes.\n\nI guess the same comment applies to the previous step, but I suspect\nthat the code structure may not allow us to switch the SSL backend\nso late in the game (e.g. \"when talking to microsoft, use schannel,\nbut when talking to github, use openssl\").\n\n> +#if LIBCURL_VERSION_NUM >= 0x072c00\n> +\t\tcurl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);\n> +#else\n> +\t\twarning(\"CURLSSLOPT_NO_REVOKE not applied to curl SSL options because\\n\"\n> +\t\t\t\"your curl version is too old (>= 7.44.0)\");\n> +#endif\n\nThat \">=\" is hard to grok. I think you meant it to be pronounced\n\"requries at least\", but that is not a common reading. People more\ncommonly pronounce it \"is greater than or equal to\".\n\n> +\t}\n> +\n> \tif (http_proactive_auth)\n> \t\tinit_curl_http_auth(result);\n"},{"id":"360587","messageId":"20181016063323.GA25933@sigill.intra.peff.net","threadId":"49568","inReplyTo":"xmqq1s8q34g2.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/3] http: add support for disabling SSL revocation checks in cURL","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-16T06:33:23Z","receivedAt":"2018-10-16T06:33:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 16, 2018 at 01:23:25PM +0900, Junio C Hamano wrote:\n\n> > +#if LIBCURL_VERSION_NUM >= 0x072c00\n> > +\t\tcurl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);\n> > +#else\n> > +\t\twarning(\"CURLSSLOPT_NO_REVOKE not applied to curl SSL options because\\n\"\n> > +\t\t\t\"your curl version is too old (>= 7.44.0)\");\n> > +#endif\n> \n> That \">=\" is hard to grok. I think you meant it to be pronounced\n> \"requries at least\", but that is not a common reading. People more\n> commonly pronounce it \"is greater than or equal to\".\n\nThis seemed oddly familiar:\n\n https://public-inbox.org/git/8da9d436-88b9-7959-dd9c-65bdd376bf54@mail.mipt.ru/\n\nSince this one is clearly copied from there, it may be worth fixing the\noriginal.\n\n-Peff\n"},{"id":"360611","messageId":"nycvar.QRO.7.76.6.1810161421340.4546@tvgsbejvaqbjf.bet","threadId":"49568","inReplyTo":"CAPig+cQFb3S0Lm+huUZDN4aw9rWwinh0iZp12ss1zVKpJ=2MdA@mail.gmail.com","subject":"Re: [PATCH 2/3] http: add support for disabling SSL revocation checks in cURL","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-16T12:21:48Z","receivedAt":"2018-10-16T12:22:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Eric,\n\n\nOn Mon, 15 Oct 2018, Eric Sunshine wrote:\n\n> On Mon, Oct 15, 2018 at 6:14 AM Brendan Forster via GitGitGadget\n> wrote:\n> > This config value is only used if http.sslBackend is set to \"schannel\",\n> > which forces cURL to use the Windows Certificate Store when validating\n> > server certificates associated with a remote server.\n> >\n> > This is only supported in cURL 7.44 or later.\n> > [...]\n> > Signed-off-by: Brendan Forster \n> > ---\n> > diff --git a/http.c b/http.c\n> > @@ -811,6 +818,16 @@ static CURL *get_curl_handle(void)\n> > + if (http_ssl_backend && !strcmp(\"schannel\", http_ssl_backend) &&\n> > + !http_schannel_check_revoke) {\n> > +#if LIBCURL_VERSION_NUM >= 0x072c00\n> > + curl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);\n> > +#else\n> > + warning(\"CURLSSLOPT_NO_REVOKE not applied to curl SSL options because\\n\"\n> > + \"your curl version is too old (>= 7.44.0)\");\n> \n> This message is confusing. If your curl is too old, shouldn't the \">=\" be a \"<\"?\n\nAbsolutely correct. Will fix,\nDscho\n\n> \n> > +#endif\n> > + }\n> \n"},{"id":"360612","messageId":"nycvar.QRO.7.76.6.1810161225310.4546@tvgsbejvaqbjf.bet","threadId":"49568","inReplyTo":"xmqq1s8q34g2.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/3] http: add support for disabling SSL revocation checks in cURL","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-16T12:22:03Z","receivedAt":"2018-10-16T12:22:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Tue, 16 Oct 2018, Junio C Hamano wrote:\n\n> \"Brendan Forster via GitGitGadget\" writes:\n> \n> > Note: an earlier iteration tried to use the config setting\n> > http.schannel.checkRevoke, but the http.* config settings can be limited\n> > to specific URLs via http..* (which would mistake `schannel` for a\n> > URL).\n> \n> Yeah, \"http.schannel.anything\" would not work, but is this note\n> relevant here? As far as the git development community as a whole\n> is concerned, this is the first iteration of the patch we see and\n> review.\n\nI did review that commit message before typing /submit, and I figured that\nit would still make sense to leave a comment there why we did *not* use\nhttp.schannel.checkRevoke. I know that *my* review comments would include\na question about this, had I not been part of the development of this\npatch.\n\n> In any case, you can use \"http..$variable\" to say \"I want the\n> http.$variable to be in effect but only when I am talking to \".\n> Does it make sense for this new variable, too? That is, does it\n> benefit the users to be able to do something like this?\n> \n> [http] schannelCheckRevoke = no\n> [http \"https://microsoft.com/\"] schannelCheckRevoke = yes\n> \n> I am guessing that the answer is yes.\n\nFrankly, I do not know. Does it hurt, though?\n\nThis patch neither introduces the `http..` feature nor prevents\nit. Its original design (which I found more logical than the current one),\nhowever, clashes with that feature.\n\n> I guess the same comment applies to the previous step, but I suspect\n> that the code structure may not allow us to switch the SSL backend\n> so late in the game (e.g. \"when talking to microsoft, use schannel,\n> but when talking to github, use openssl\").\n\nThat's a really good question, and I suspect that it is actually not too\nlate. Let me try.\n\n*clicketyclick*\n\n-- snip --\n$ git config --show-origin http.sslbackend\nfile:C:/Program Files/Git/mingw64/etc/gitconfig schannel\n\n$ GIT_TRACE_CURL=1 git -c 'http.https://github.com/dscho/images.sslbackend=openssl' ls-remote https://github.com/dscho/images\n14:03:52.319986 http.c:724 == Info: Couldn't find host github.com in the _netrc file; using defaults\n14:03:52.366858 http.c:724 == Info: Trying 192.30.253.113...\n14:03:52.366858 http.c:724 == Info: TCP_NODELAY set\n14:03:52.482825 http.c:724 == Info: Connected to github.com (192.30.253.113) port 443 (#0)\n14:03:52.721173 http.c:724 == Info: ALPN, offering http/1.1\n14:03:52.721173 http.c:724 == Info: Cipher selection: ALL:!EXPORT:!EXPORT40:!EXPORT56:!aNULL:!LOW:!RC4:@STRENGTH\n14:03:52.721173 http.c:724 == Info: error setting certificate verify locations:\n CAfile: C:/Program Files/Git/mingw64/libexec/ssl/certs/ca-bundle.crt\n CApath: none\nfatal: unable to access 'https://github.com/dscho/images/': error setting certificate verify locations:\n CAfile: C:/Program Files/Git/mingw64/libexec/ssl/certs/ca-bundle.crt\n CApath: none\n-- snap --\n\nPlease allow me to help understand this log. First, I verified that the\ncurrently-configured backend is Secure Channel. Then, I ask Git to list\nthe remote refs of a small repository, special-casing it to the OpenSSL\nbackend.\n\nCrucially, this fails. The short version is: this is good! Because it\nmeans that Git used the OpenSSL backend, as clearly intended.\n\n\nWhy does it fail?\n\nTwo reasons:\n\n1) Git for Windows has to disable the certificate bundle. The gist of it\nis: Git LFS uses Git for Windows' certificate bundle, if configured, and\nthat would override the Windows Certificate Store. When users configure\nSecure Channel, however, they *want* to use the Windows Certificate Store,\nso to accommodate Git LFS, we \"unconfigure\" http.sslCAInfo in that case.\n\n2) The libcurl used by Git for Windows has some smarts built in to\nunderstand relative paths to its \"Unix pseudo root\". However, this fails\nwhen libcurl is loaded from libexec/git-core/ (which is the case, to avoid\nany libcurl-4.dll in C:\\Windows\\system32 from being picked up by mistake).\n\nIf this explanation sounds obscure, the reason is that it *is* obscure. If\nyou are truly interested in the details, I can point you to the relevant\ntickets on Git for Windows' bug tracker.\n\n\n> > +#if LIBCURL_VERSION_NUM >= 0x072c00\n> > +\t\tcurl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);\n> > +#else\n> > +\t\twarning(\"CURLSSLOPT_NO_REVOKE not applied to curl SSL options because\\n\"\n> > +\t\t\t\"your curl version is too old (>= 7.44.0)\");\n> > +#endif\n> \n> That \">=\" is hard to grok. I think you meant it to be pronounced\n> \"requries at least\", but that is not a common reading. People more\n> commonly pronounce it \"is greater than or equal to\".\n\nEric made the same point. I will change it to '<'.\n\nSide note: this is the kind of change that I am completely comfortable\nmaking, even on patches that were battle-tested, because those kind of\nchanges are certain not to affect the correctness of the patch.\n\nThanks,\nDscho\n\n> \n> > +\t}\n> > +\n> > \tif (http_proactive_auth)\n> > \t\tinit_curl_http_auth(result);\n> \n"},{"id":"360613","messageId":"nycvar.QRO.7.76.6.1810161424390.4546@tvgsbejvaqbjf.bet","threadId":"49568","inReplyTo":"20181016063323.GA25933@sigill.intra.peff.net","subject":"Re: [PATCH 2/3] http: add support for disabling SSL revocation checks in cURL","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-16T12:25:57Z","receivedAt":"2018-10-16T12:26:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Tue, 16 Oct 2018, Jeff King wrote:\n\n> On Tue, Oct 16, 2018 at 01:23:25PM +0900, Junio C Hamano wrote:\n> \n> > > +#if LIBCURL_VERSION_NUM >= 0x072c00\n> > > +\t\tcurl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);\n> > > +#else\n> > > +\t\twarning(\"CURLSSLOPT_NO_REVOKE not applied to curl SSL options because\\n\"\n> > > +\t\t\t\"your curl version is too old (>= 7.44.0)\");\n> > > +#endif\n> > \n> > That \">=\" is hard to grok. I think you meant it to be pronounced\n> > \"requries at least\", but that is not a common reading. People more\n> > commonly pronounce it \"is greater than or equal to\".\n> \n> This seemed oddly familiar:\n> \n> https://public-inbox.org/git/8da9d436-88b9-7959-dd9c-65bdd376bf54@mail.mipt.ru/\n> \n> Since this one is clearly copied from there, it may be worth fixing the\n> original.\n\nGood memory. I just integrated the patch here. It was not signed off, but\nit is too obvious to be protected under copyright, so I re-did it, adding\na nice commit message.\n\nCiao,\nDscho\n"},{"id":"360632","messageId":"20181016152831.GA29432@sigill.intra.peff.net","threadId":"49568","inReplyTo":"nycvar.QRO.7.76.6.1810161424390.4546@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/3] http: add support for disabling SSL revocation checks in cURL","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-16T15:28:31Z","receivedAt":"2018-10-16T15:28:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 16, 2018 at 02:25:57PM +0200, Johannes Schindelin wrote:\n\n> > > That \">=\" is hard to grok. I think you meant it to be pronounced\n> > > \"requries at least\", but that is not a common reading. People more\n> > > commonly pronounce it \"is greater than or equal to\".\n> > \n> > This seemed oddly familiar:\n> > \n> > https://public-inbox.org/git/8da9d436-88b9-7959-dd9c-65bdd376bf54@mail.mipt.ru/\n> > \n> > Since this one is clearly copied from there, it may be worth fixing the\n> > original.\n> \n> Good memory. I just integrated the patch here. It was not signed off, but\n> it is too obvious to be protected under copyright, so I re-did it, adding\n> a nice commit message.\n\nYay, thank you (I considered doing the same, but was scratching my head\nover how to deal with authorship and signoff).\n\n-Peff\n"},{"id":"360801","messageId":"xmqq1s8oxbpc.fsf@gitster-ct.c.googlers.com","threadId":"49568","inReplyTo":"nycvar.QRO.7.76.6.1810161225310.4546@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/3] http: add support for disabling SSL revocation checks in cURL","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-18T01:53:03Z","receivedAt":"2018-10-18T01:53:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> In any case, you can use \"http..$variable\" to say \"I want the\n>> http.$variable to be in effect but only when I am talking to \".\n>> Does it make sense for this new variable, too? That is, does it\n>> benefit the users to be able to do something like this?\n>> \n>> [http] schannelCheckRevoke = no\n>> [http \"https://microsoft.com/\"] schannelCheckRevoke = yes\n>> \n>> I am guessing that the answer is yes.\n>\n> Frankly, I do not know. Does it hurt, though?\n\nI did not and I do not think it would. I was wondering if the\nability to be able to specify these per destination is something\nvery useful and deserves to be called out in the doc, together with\n...\n\n>> I guess the same comment applies to the previous step, but I suspect\n>> that the code structure may not allow us to switch the SSL backend\n>> so late in the game (e.g. \"when talking to microsoft, use schannel,\n>> but when talking to github, use openssl\").\n\n... this bit.\n\n> Crucially, this fails. The short version is: this is good! Because it\n> means that Git used the OpenSSL backend, as clearly intended.\n>\n> \n> Why does it fail?\n> ...\n> \n\nSo there may still be some polishing needed, but as long as people\nare not using the \"per destination\" thing, the code is already good?\nThat is something we may want to document.\n\nThanks.\n"},{"id":"361462","messageId":"xmqqzhv2lnn6.fsf@gitster-ct.c.googlers.com","threadId":"49568","inReplyTo":"CAPig+cQFb3S0Lm+huUZDN4aw9rWwinh0iZp12ss1zVKpJ=2MdA@mail.gmail.com","subject":"Re: [PATCH 2/3] http: add support for disabling SSL revocation checks in cURL","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-25T03:18:37Z","receivedAt":"2018-10-25T03:18:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Mon, Oct 15, 2018 at 6:14 AM Brendan Forster via GitGitGadget\n> wrote:\n>> This config value is only used if http.sslBackend is set to \"schannel\",\n>> which forces cURL to use the Windows Certificate Store when validating\n>> server certificates associated with a remote server.\n>>\n>> This is only supported in cURL 7.44 or later.\n>> [...]\n>> Signed-off-by: Brendan Forster \n>> ---\n>> diff --git a/http.c b/http.c\n>> @@ -811,6 +818,16 @@ static CURL *get_curl_handle(void)\n>> + if (http_ssl_backend && !strcmp(\"schannel\", http_ssl_backend) &&\n>> + !http_schannel_check_revoke) {\n>> +#if LIBCURL_VERSION_NUM >= 0x072c00\n>> + curl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);\n>> +#else\n>> + warning(\"CURLSSLOPT_NO_REVOKE not applied to curl SSL options because\\n\"\n>> + \"your curl version is too old (>= 7.44.0)\");\n>\n> This message is confusing. If your curl is too old, shouldn't the \">=\" be a \"<\"?\n\nI do not think I saw any update to correct this, and worse yet I do\nnot offhand recall if there was any other issue raised on the\nseries.\n\nSo assuming that this is the only remaining one, I'll squash the\nfollowing to step 2/3 of this three-patch series and plan to merge\nit down to 'next' in the coming few days.\n\nI have a clean-up suggestion related to this but is orthogonal to\nthis three-patch series (after the fix-up is applied, anyway), which\nI'll be sending out separately.\n\nThanks.\n\n http.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/http.c b/http.c\nindex 0ebf8f77a6..43e75ac583 100644\n--- a/http.c\n+++ b/http.c\n@@ -835,7 +835,7 @@ static CURL *get_curl_handle(void)\n \t\tcurl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);\n #else\n \t\twarning(\"CURLSSLOPT_NO_REVOKE not applied to curl SSL options because\\n\"\n-\t\t\t\"your curl version is too old (>= 7.44.0)\");\n+\t\t\t\"your curl version is too old (< 7.44.0)\");\n #endif\n \t}\n \n-- \n2.19.1-542-gc4df23f792\n\n"},{"id":"361463","messageId":"xmqqsh0uln5c.fsf_-_@gitster-ct.c.googlers.com","threadId":"49568","inReplyTo":"xmqqzhv2lnn6.fsf@gitster-ct.c.googlers.com","subject":"[PATCH] http: give curl version warnings consistently","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-25T03:29:19Z","receivedAt":"2018-10-25T03:29:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When a requested feature cannot be activated because the version of\ncURL library used to build Git with is too old, most of the codepaths\ngive a warning like \"$Feature is not supported with cURL < $Version\",\nmarked for l10n. A few of them, however, did not follow that pattern\nand said things like \"$Feature is not activated, your curl version is\ntoo old (>= $Version)\", and without marking them for l10n.\n\nUpdate these to match the style of the majority of warnings and mark\nthem for l10n.\n\nSigned-off-by: Junio C Hamano \n---\n\n > I have a clean-up suggestion related to this but is orthogonal to\n > this three-patch series (after the fix-up is applied, anyway), which\n > I'll be sending out separately.\n\n So, here it is.\n\n http.c | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 43e75ac583..2214100e3b 100644\n--- a/http.c\n+++ b/http.c\n@@ -834,8 +834,7 @@ static CURL *get_curl_handle(void)\n #if LIBCURL_VERSION_NUM >= 0x072c00\n \t\tcurl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);\n #else\n-\t\twarning(\"CURLSSLOPT_NO_REVOKE not applied to curl SSL options because\\n\"\n-\t\t\t\"your curl version is too old (< 7.44.0)\");\n+\t\twarning(_(\"CURLSSLOPT_NO_REVOKE not suported with cURL < 7.44.0\"));\n #endif\n \t}\n \n@@ -908,8 +907,7 @@ static CURL *get_curl_handle(void)\n \tcurl_easy_setopt(result, CURLOPT_PROTOCOLS,\n \t\t\t get_curl_allowed_protocols(-1));\n #else\n-\twarning(\"protocol restrictions not applied to curl redirects because\\n\"\n-\t\t\"your curl version is too old (>= 7.19.4)\");\n+\twarning(_(\"Protocol restrictions not supported with cURL < 7.19.4\"));\n #endif\n \tif (getenv(\"GIT_CURL_VERBOSE\"))\n \t\tcurl_easy_setopt(result, CURLOPT_VERBOSE, 1L);\n-- \n2.19.1-542-gc4df23f792\n\n"},{"id":"361480","messageId":"20181025062332.GD11460@sigill.intra.peff.net","threadId":"49568","inReplyTo":"xmqqsh0uln5c.fsf_-_@gitster-ct.c.googlers.com","subject":"Re: [PATCH] http: give curl version warnings consistently","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-25T06:23:32Z","receivedAt":"2018-10-25T06:23:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 25, 2018 at 12:29:19PM +0900, Junio C Hamano wrote:\n\n> When a requested feature cannot be activated because the version of\n> cURL library used to build Git with is too old, most of the codepaths\n> give a warning like \"$Feature is not supported with cURL < $Version\",\n> marked for l10n. A few of them, however, did not follow that pattern\n> and said things like \"$Feature is not activated, your curl version is\n> too old (>= $Version)\", and without marking them for l10n.\n> \n> Update these to match the style of the majority of warnings and mark\n> them for l10n.\n\nThis is definitely an improvement.\n\n> @@ -908,8 +907,7 @@ static CURL *get_curl_handle(void)\n> \tcurl_easy_setopt(result, CURLOPT_PROTOCOLS,\n> \t\t\t get_curl_allowed_protocols(-1));\n> #else\n> -\twarning(\"protocol restrictions not applied to curl redirects because\\n\"\n> -\t\t\"your curl version is too old (>= 7.19.4)\");\n> +\twarning(_(\"Protocol restrictions not supported with cURL < 7.19.4\"));\n\nThis loses the mention of redirects, but I think that is actually a\nbonus. The #ifdef'd code covers both original and redirected requests.\n\n-Peff\n"},{"id":"361510","messageId":"nycvar.QRO.7.76.6.1810251410360.4546@tvgsbejvaqbjf.bet","threadId":"49568","inReplyTo":"xmqqzhv2lnn6.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/3] http: add support for disabling SSL revocation checks in cURL","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-25T12:12:57Z","receivedAt":"2018-10-25T12:13:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 25 Oct 2018, Junio C Hamano wrote:\n\n> Eric Sunshine writes:\n> \n> > On Mon, Oct 15, 2018 at 6:14 AM Brendan Forster via GitGitGadget\n> > wrote:\n> >> This config value is only used if http.sslBackend is set to \"schannel\",\n> >> which forces cURL to use the Windows Certificate Store when validating\n> >> server certificates associated with a remote server.\n> >>\n> >> This is only supported in cURL 7.44 or later.\n> >> [...]\n> >> Signed-off-by: Brendan Forster \n> >> ---\n> >> diff --git a/http.c b/http.c\n> >> @@ -811,6 +818,16 @@ static CURL *get_curl_handle(void)\n> >> + if (http_ssl_backend && !strcmp(\"schannel\", http_ssl_backend) &&\n> >> + !http_schannel_check_revoke) {\n> >> +#if LIBCURL_VERSION_NUM >= 0x072c00\n> >> + curl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);\n> >> +#else\n> >> + warning(\"CURLSSLOPT_NO_REVOKE not applied to curl SSL options because\\n\"\n> >> + \"your curl version is too old (>= 7.44.0)\");\n> >\n> > This message is confusing. If your curl is too old, shouldn't the \">=\" be a \"<\"?\n> \n> I do not think I saw any update to correct this, and worse yet I do\n> not offhand recall if there was any other issue raised on the\n> series.\n\nSorry, my bad. I dropped the ball. As you can see here:\n\n\thttps://github.com/gitgitgadget/git/pull/46\n\nI have some updates that are already pushed, but I still wanted to really\nthink through your response here:\n\n\thttps://public-inbox.org/git/xmqq1s8oxbpc.fsf@gitster-ct.c.googlers.com/\n\nand what I should do about it, before sending off v2. You can see that I\nalready updated the description in preparation for sending another\niteration.\n\nI hope to get back to this tonight, for now I must scramble off to\nnon-work-related activities.\n\nCiao,\nDscho\n\n> So assuming that this is the only remaining one, I'll squash the\n> following to step 2/3 of this three-patch series and plan to merge\n> it down to 'next' in the coming few days.\n> \n> I have a clean-up suggestion related to this but is orthogonal to\n> this three-patch series (after the fix-up is applied, anyway), which\n> I'll be sending out separately.\n> \n> Thanks.\n> \n> http.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/http.c b/http.c\n> index 0ebf8f77a6..43e75ac583 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -835,7 +835,7 @@ static CURL *get_curl_handle(void)\n> \t\tcurl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);\n> #else\n> \t\twarning(\"CURLSSLOPT_NO_REVOKE not applied to curl SSL options because\\n\"\n> -\t\t\t\"your curl version is too old (>= 7.44.0)\");\n> +\t\t\t\"your curl version is too old (< 7.44.0)\");\n> #endif\n> \t}\n> \n> -- \n> 2.19.1-542-gc4df23f792\n> \n> \n"},{"id":"361541","messageId":"nycvar.QRO.7.76.6.1810252049210.4546@tvgsbejvaqbjf.bet","threadId":"49568","inReplyTo":"xmqq1s8oxbpc.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/3] http: add support for disabling SSL revocation checks in cURL","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-25T18:52:16Z","receivedAt":"2018-10-25T18:52:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 18 Oct 2018, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> >> In any case, you can use \"http..$variable\" to say \"I want the\n> >> http.$variable to be in effect but only when I am talking to \".\n> >> Does it make sense for this new variable, too? That is, does it\n> >> benefit the users to be able to do something like this?\n> >> \n> >> [http] schannelCheckRevoke = no\n> >> [http \"https://microsoft.com/\"] schannelCheckRevoke = yes\n> >> \n> >> I am guessing that the answer is yes.\n> >\n> > Frankly, I do not know. Does it hurt, though?\n> \n> I did not and I do not think it would. I was wondering if the\n> ability to be able to specify these per destination is something\n> very useful and deserves to be called out in the doc, together with\n> ...\n\nI do not think that it needs to be called out specifically in the docs. It\nis just yet another http.* setting that can be overridden per-URL. It\nwould be different if it had not worked.\n\n> >> I guess the same comment applies to the previous step, but I suspect\n> >> that the code structure may not allow us to switch the SSL backend\n> >> so late in the game (e.g. \"when talking to microsoft, use schannel,\n> >> but when talking to github, use openssl\").\n> \n> ... this bit.\n> \n> > Crucially, this fails. The short version is: this is good! Because it\n> > means that Git used the OpenSSL backend, as clearly intended.\n> >\n> > \n> > Why does it fail?\n> > ...\n> > \n> \n> So there may still be some polishing needed, but as long as people\n> are not using the \"per destination\" thing, the code is already good?\n> That is something we may want to document.\n\nActually, just because there is a peculiar bug in this particular code\nflow in Git for Windows should not necessarily be interesting to Git's\ncommit history.\n\nOn Linux, for example, it would work.\n\nSo I don't think we need to mention anything about that unrelated bug.\n\nThanks,\nDscho\n\n> Thanks.\n> \n"},{"id":"361542","messageId":"pull.46.v2.git.gitgitgadget@gmail.com","threadId":"49568","inReplyTo":"pull.46.git.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] Allow choosing the SSL backend cURL uses (plus related patches)","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-10-25T18:53:52Z","receivedAt":"2018-10-25T18:53:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This topic branch brings support for choosing cURL's SSL backend (a feature\ndeveloped in Git for Windows' context) at runtime via http.sslBackend, and\ntwo more patches that are related (and only of interest for Windows users).\n\nChanges since v1:\n\n * Reworded the commit message of v1's patch 2/3, to talk about the original\n design instead of \"an earlier iteration\" that was never contributed to\n the Git mailing list.\n * Changed the confusing >= 7.44.0 to < 7.44.0.\n\nNote: I had prepared \nhttps://github.com/dscho/git/commit/81e8c9a4006c919747a0b6a287f28f25799fcaf4\n, intended to be included in v2, but Junio came up with \nhttps://public-inbox.org/git/xmqqsh0uln5c.fsf_-_@gitster-ct.c.googlers.com/ \nin the meantime, which I like better.\n\nBrendan Forster (1):\n http: add support for disabling SSL revocation checks in cURL\n\nJohannes Schindelin (2):\n http: add support for selecting SSL backends at runtime\n http: when using Secure Channel, ignore sslCAInfo by default\n\n Documentation/config.txt | 21 ++++++++++++\n http.c | 71 +++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 91 insertions(+), 1 deletion(-)\n\n\nbase-commit: 5a0cc8aca797dbd7d2be3b67458ff880ed45cddf\nPublished-As: https://github.com/gitgitgadget/git/releases/tags/pr-46%2Fdscho%2Fhttp-ssl-backend-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-46/dscho/http-ssl-backend-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/46\n\nRange-diff vs v1:\n\n 1: 8c5ecdb6c = 1: 85bd0fb27 http: add support for selecting SSL backends at runtime\n 2: 764791d13 ! 2: 951383695 http: add support for disabling SSL revocation checks in cURL\n @@ -14,10 +14,10 @@\n \n This is only supported in cURL 7.44 or later.\n \n - Note: an earlier iteration tried to use the config setting\n - http.schannel.checkRevoke, but the http.* config settings can be limited\n - to specific URLs via http..* (which would mistake `schannel` for a\n - URL).\n + Note: originally, we wanted to call the config setting\n + `http.schannel.checkRevoke`. This, however, does not work: the `http.*`\n + config settings can be limited to specific URLs via `http..*`\n + (and this feature would mistake `schannel` for a URL).\n \n Helped by Agustín Martín Barbero.\n \n @@ -77,7 +77,7 @@\n +\t\tcurl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);\n +#else\n +\t\twarning(\"CURLSSLOPT_NO_REVOKE not applied to curl SSL options because\\n\"\n -+\t\t\t\"your curl version is too old (>= 7.44.0)\");\n ++\t\t\t\"your curl version is too old (< 7.44.0)\");\n +#endif\n +\t}\n +\n 3: 9927e4ce6 = 3: a5f937a36 http: when using Secure Channel, ignore sslCAInfo by default\n\n-- \ngitgitgadget\n"},{"id":"361543","messageId":"85bd0fb27fcf7615b3f927344fd77ea49b9f5dcb.1540493630.git.gitgitgadget@gmail.com","threadId":"49568","inReplyTo":"pull.46.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] http: add support for selecting SSL backends at runtime","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-10-25T18:53:53Z","receivedAt":"2018-10-25T18:53:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nAs of version 7.56.0, curl supports being compiled with multiple SSL\nbackends.\n\nThis patch adds the Git side of that feature: by setting http.sslBackend\nto \"openssl\" or \"schannel\", Git for Windows can now choose the SSL\nbackend at runtime.\n\nThis comes in handy on Windows because Secure Channel (\"schannel\") is\nthe native solution, accessing the Windows Credential Store, thereby\nallowing for enterprise-wide management of certificates. For historical\nreasons, Git for Windows needs to support OpenSSL still, as it has\npreviously been the only supported SSL backend in Git for Windows for\nalmost a decade.\n\nThe patch has been carried in Git for Windows for over a year, and is\nconsidered mature.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/config.txt | 5 +++++\n http.c | 35 +++++++++++++++++++++++++++++++++++\n 2 files changed, 40 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 154683321..7d38f0bf1 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1984,6 +1984,11 @@ http.sslCAPath::\n \twith when fetching or pushing over HTTPS. Can be overridden\n \tby the `GIT_SSL_CAPATH` environment variable.\n \n+http.sslBackend::\n+\tName of the SSL backend to use (e.g. \"openssl\" or \"schannel\").\n+\tThis option is ignored if cURL lacks support for choosing the SSL\n+\tbackend at runtime.\n+\n http.pinnedpubkey::\n \tPublic key of the https service. It may either be the filename of\n \ta PEM or DER encoded public key file or a string starting with\ndiff --git a/http.c b/http.c\nindex 98ff12258..7fb37a061 100644\n--- a/http.c\n+++ b/http.c\n@@ -155,6 +155,8 @@ static struct active_request_slot *active_queue_head;\n \n static char *cached_accept_language;\n \n+static char *http_ssl_backend;\n+\n size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)\n {\n \tsize_t size = eltsize * nmemb;\n@@ -302,6 +304,12 @@ static int http_options(const char *var, const char *value, void *cb)\n \t\tcurl_ssl_try = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(\"http.sslbackend\", var)) {\n+\t\tfree(http_ssl_backend);\n+\t\thttp_ssl_backend = xstrdup_or_null(value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(\"http.minsessions\", var)) {\n \t\tmin_curl_sessions = git_config_int(var, value);\n #ifndef USE_CURL_MULTI\n@@ -995,6 +1003,33 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)\n \tgit_config(urlmatch_config_entry, &config);\n \tfree(normalized_url);\n \n+#if LIBCURL_VERSION_NUM >= 0x073800\n+\tif (http_ssl_backend) {\n+\t\tconst curl_ssl_backend **backends;\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tint i;\n+\n+\t\tswitch (curl_global_sslset(-1, http_ssl_backend, &backends)) {\n+\t\tcase CURLSSLSET_UNKNOWN_BACKEND:\n+\t\t\tstrbuf_addf(&buf, _(\"Unsupported SSL backend '%s'. \"\n+\t\t\t\t\t \"Supported SSL backends:\"),\n+\t\t\t\t\t http_ssl_backend);\n+\t\t\tfor (i = 0; backends[i]; i++)\n+\t\t\t\tstrbuf_addf(&buf, \"\\n\\t%s\", backends[i]->name);\n+\t\t\tdie(\"%s\", buf.buf);\n+\t\tcase CURLSSLSET_NO_BACKENDS:\n+\t\t\tdie(_(\"Could not set SSL backend to '%s': \"\n+\t\t\t \"cURL was built without SSL backends\"),\n+\t\t\t http_ssl_backend);\n+\t\tcase CURLSSLSET_TOO_LATE:\n+\t\t\tdie(_(\"Could not set SSL backend to '%s': already set\"),\n+\t\t\t http_ssl_backend);\n+\t\tcase CURLSSLSET_OK:\n+\t\t\tbreak; /* Okay! */\n+\t\t}\n+\t}\n+#endif\n+\n \tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n \t\tdie(\"curl_global_init failed\");\n \n-- \ngitgitgadget\n\n"},{"id":"361544","messageId":"951383695d9c9a30a11fcf3dc6a268e604d10f3a.1540493630.git.gitgitgadget@gmail.com","threadId":"49568","inReplyTo":"pull.46.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] http: add support for disabling SSL revocation checks in cURL","fromName":"Brendan Forster via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-10-25T18:53:55Z","receivedAt":"2018-10-25T18:53:57Z","isPatch":true,"sender":{"key":"name:Brendan Forster","avatar":null},"body":"From: Brendan Forster \n\nThis adds support for a new http.schannelCheckRevoke config value.\n\nThis config value is only used if http.sslBackend is set to \"schannel\",\nwhich forces cURL to use the Windows Certificate Store when validating\nserver certificates associated with a remote server.\n\nThis config value should only be set to \"false\" if you are in an\nenvironment where revocation checks are blocked by the network, with\nno alternative options.\n\nThis is only supported in cURL 7.44 or later.\n\nNote: originally, we wanted to call the config setting\n`http.schannel.checkRevoke`. This, however, does not work: the `http.*`\nconfig settings can be limited to specific URLs via `http..*`\n(and this feature would mistake `schannel` for a URL).\n\nHelped by Agustín Martín Barbero.\n\nSigned-off-by: Brendan Forster \nSigned-off-by: Johannes Schindelin \n---\n Documentation/config.txt | 8 ++++++++\n http.c | 17 +++++++++++++++++\n 2 files changed, 25 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 7d38f0bf1..d569ebd49 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1989,6 +1989,14 @@ http.sslBackend::\n \tThis option is ignored if cURL lacks support for choosing the SSL\n \tbackend at runtime.\n \n+http.schannelCheckRevoke::\n+\tUsed to enforce or disable certificate revocation checks in cURL\n+\twhen http.sslBackend is set to \"schannel\". Defaults to `true` if\n+\tunset. Only necessary to disable this if Git consistently errors\n+\tand the message is about checking the revocation status of a\n+\tcertificate. This option is ignored if cURL lacks support for\n+\tsetting the relevant SSL option at runtime.\n+\n http.pinnedpubkey::\n \tPublic key of the https service. It may either be the filename of\n \ta PEM or DER encoded public key file or a string starting with\ndiff --git a/http.c b/http.c\nindex 7fb37a061..65daa9bfa 100644\n--- a/http.c\n+++ b/http.c\n@@ -157,6 +157,8 @@ static char *cached_accept_language;\n \n static char *http_ssl_backend;\n \n+static int http_schannel_check_revoke = 1;\n+\n size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)\n {\n \tsize_t size = eltsize * nmemb;\n@@ -310,6 +312,11 @@ static int http_options(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(\"http.schannelcheckrevoke\", var)) {\n+\t\thttp_schannel_check_revoke = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(\"http.minsessions\", var)) {\n \t\tmin_curl_sessions = git_config_int(var, value);\n #ifndef USE_CURL_MULTI\n@@ -811,6 +818,16 @@ static CURL *get_curl_handle(void)\n \t}\n #endif\n \n+\tif (http_ssl_backend && !strcmp(\"schannel\", http_ssl_backend) &&\n+\t !http_schannel_check_revoke) {\n+#if LIBCURL_VERSION_NUM >= 0x072c00\n+\t\tcurl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);\n+#else\n+\t\twarning(\"CURLSSLOPT_NO_REVOKE not applied to curl SSL options because\\n\"\n+\t\t\t\"your curl version is too old (< 7.44.0)\");\n+#endif\n+\t}\n+\n \tif (http_proactive_auth)\n \t\tinit_curl_http_auth(result);\n \n-- \ngitgitgadget\n\n"},{"id":"361545","messageId":"a5f937a36d454d6d99bb64d4be3c389cf996826a.1540493630.git.gitgitgadget@gmail.com","threadId":"49568","inReplyTo":"pull.46.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] http: when using Secure Channel, ignore sslCAInfo by default","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-10-25T18:53:56Z","receivedAt":"2018-10-25T18:53:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nAs of cURL v7.60.0, the Secure Channel backend can use the certificate\nbundle provided via `http.sslCAInfo`, but that would override the\nWindows Certificate Store. Since this is not desirable by default, let's\ntell Git to not ask cURL to use that bundle by default when the `schannel`\nbackend was configured via `http.sslBackend`, unless\n`http.schannelUseSSLCAInfo` overrides this behavior.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/config.txt | 8 ++++++++\n http.c | 19 ++++++++++++++++++-\n 2 files changed, 26 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex d569ebd49..1f6a6a4a6 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1997,6 +1997,14 @@ http.schannelCheckRevoke::\n \tcertificate. This option is ignored if cURL lacks support for\n \tsetting the relevant SSL option at runtime.\n \n+http.schannelUseSSLCAInfo::\n+\tAs of cURL v7.60.0, the Secure Channel backend can use the\n+\tcertificate bundle provided via `http.sslCAInfo`, but that would\n+\toverride the Windows Certificate Store. Since this is not desirable\n+\tby default, Git will tell cURL not to use that bundle by default\n+\twhen the `schannel` backend was configured via `http.sslBackend`,\n+\tunless `http.schannelUseSSLCAInfo` overrides this behavior.\n+\n http.pinnedpubkey::\n \tPublic key of the https service. It may either be the filename of\n \ta PEM or DER encoded public key file or a string starting with\ndiff --git a/http.c b/http.c\nindex 65daa9bfa..28009ca73 100644\n--- a/http.c\n+++ b/http.c\n@@ -158,6 +158,12 @@ static char *cached_accept_language;\n static char *http_ssl_backend;\n \n static int http_schannel_check_revoke = 1;\n+/*\n+ * With the backend being set to `schannel`, setting sslCAinfo would override\n+ * the Certificate Store in cURL v7.60.0 and later, which is not what we want\n+ * by default.\n+ */\n+static int http_schannel_use_ssl_cainfo;\n \n size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)\n {\n@@ -317,6 +323,11 @@ static int http_options(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(\"http.schannelusesslcainfo\", var)) {\n+\t\thttp_schannel_use_ssl_cainfo = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(\"http.minsessions\", var)) {\n \t\tmin_curl_sessions = git_config_int(var, value);\n #ifndef USE_CURL_MULTI\n@@ -869,7 +880,13 @@ static CURL *get_curl_handle(void)\n \tif (ssl_pinnedkey != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_PINNEDPUBLICKEY, ssl_pinnedkey);\n #endif\n-\tif (ssl_cainfo != NULL)\n+\tif (http_ssl_backend && !strcmp(\"schannel\", http_ssl_backend) &&\n+\t !http_schannel_use_ssl_cainfo) {\n+\t\tcurl_easy_setopt(result, CURLOPT_CAINFO, NULL);\n+#if LIBCURL_VERSION_NUM >= 0x073400\n+\t\tcurl_easy_setopt(result, CURLOPT_PROXY_CAINFO, NULL);\n+#endif\n+\t} else if (ssl_cainfo != NULL)\n \t\tcurl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);\n \n \tif (curl_low_speed_limit > 0 && curl_low_speed_time > 0) {\n-- \ngitgitgadget\n"},{"id":"361548","messageId":"nycvar.QRO.7.76.6.1810252059510.4546@tvgsbejvaqbjf.bet","threadId":"49568","inReplyTo":"xmqqsh0uln5c.fsf_-_@gitster-ct.c.googlers.com","subject":"Re: [PATCH] http: give curl version warnings consistently","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-25T19:00:32Z","receivedAt":"2018-10-25T19:00:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 25 Oct 2018, Junio C Hamano wrote:\n\n> When a requested feature cannot be activated because the version of\n> cURL library used to build Git with is too old, most of the codepaths\n> give a warning like \"$Feature is not supported with cURL < $Version\",\n> marked for l10n. A few of them, however, did not follow that pattern\n> and said things like \"$Feature is not activated, your curl version is\n> too old (>= $Version)\", and without marking them for l10n.\n> \n> Update these to match the style of the majority of warnings and mark\n> them for l10n.\n> \n> Signed-off-by: Junio C Hamano \n> ---\n\nI like this patch better than the one I had prepared for v2, so I dropped\nit again, and \"hit the Submit button\".\n\nCiao,\nDscho\n\n> \n> > I have a clean-up suggestion related to this but is orthogonal to\n> > this three-patch series (after the fix-up is applied, anyway), which\n> > I'll be sending out separately.\n> \n> So, here it is.\n> \n> http.c | 6 ++----\n> 1 file changed, 2 insertions(+), 4 deletions(-)\n> \n> diff --git a/http.c b/http.c\n> index 43e75ac583..2214100e3b 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -834,8 +834,7 @@ static CURL *get_curl_handle(void)\n> #if LIBCURL_VERSION_NUM >= 0x072c00\n> \t\tcurl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);\n> #else\n> -\t\twarning(\"CURLSSLOPT_NO_REVOKE not applied to curl SSL options because\\n\"\n> -\t\t\t\"your curl version is too old (< 7.44.0)\");\n> +\t\twarning(_(\"CURLSSLOPT_NO_REVOKE not suported with cURL < 7.44.0\"));\n> #endif\n> \t}\n> \n> @@ -908,8 +907,7 @@ static CURL *get_curl_handle(void)\n> \tcurl_easy_setopt(result, CURLOPT_PROTOCOLS,\n> \t\t\t get_curl_allowed_protocols(-1));\n> #else\n> -\twarning(\"protocol restrictions not applied to curl redirects because\\n\"\n> -\t\t\"your curl version is too old (>= 7.19.4)\");\n> +\twarning(_(\"Protocol restrictions not supported with cURL < 7.19.4\"));\n> #endif\n> \tif (getenv(\"GIT_CURL_VERBOSE\"))\n> \t\tcurl_easy_setopt(result, CURLOPT_VERBOSE, 1L);\n> -- \n> 2.19.1-542-gc4df23f792\n> \n> \n"},{"id":"361595","messageId":"xmqqwoq5coex.fsf@gitster-ct.c.googlers.com","threadId":"49568","inReplyTo":"nycvar.QRO.7.76.6.1810252059510.4546@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] http: give curl version warnings consistently","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-26T04:39:02Z","receivedAt":"2018-10-26T04:39:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Thu, 25 Oct 2018, Junio C Hamano wrote:\n>\n>> When a requested feature cannot be activated because the version of\n>> cURL library used to build Git with is too old, most of the codepaths\n>> give a warning like \"$Feature is not supported with cURL < $Version\",\n>> marked for l10n. A few of them, however, did not follow that pattern\n>> and said things like \"$Feature is not activated, your curl version is\n>> too old (>= $Version)\", and without marking them for l10n.\n>> \n>> Update these to match the style of the majority of warnings and mark\n>> them for l10n.\n>> \n>> Signed-off-by: Junio C Hamano \n>> ---\n>\n> I like this patch better than the one I had prepared for v2, so I dropped\n> it again, and \"hit the Submit button\".\n\nI took your v3 and queue this on top, instead of the previous one\non which this was prepared.\n\nBy the way, I wondered if we want to unify them by introducing\n\n\tstatic void curl_version_warning(const char *feature, const char *verstring)\n\t{\n\t\twarning(_(\"%s is not supported with cURL < %s\"),\n\t\t\tfeature, verstring);\n\t}\n\nso that translators need to deal with a single instance of the\nmessage, but the \"feature\" part may have to get localized, in which\ncase we'd end up with sentence lego, which is not a good idea, so I\ndropped it.\n\nThanks.\n"},{"id":"361596","messageId":"xmqqsh0tcoas.fsf@gitster-ct.c.googlers.com","threadId":"49568","inReplyTo":"nycvar.QRO.7.76.6.1810252049210.4546@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/3] http: add support for disabling SSL revocation checks in cURL","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-26T04:41:31Z","receivedAt":"2018-10-26T04:41:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> I did not and I do not think it would. I was wondering if the\n>> ability to be able to specify these per destination is something\n>> very useful and deserves to be called out in the doc, together with\n>> ...\n>\n> I do not think that it needs to be called out specifically in the docs. It\n> is just yet another http.* setting that can be overridden per-URL. It\n> would be different if it had not worked.\n\nOK, thanks for sanity checking.\n"},{"id":"365236","messageId":"87sgz1n53e.fsf@evledraar.gmail.com","threadId":"49568","inReplyTo":"85bd0fb27fcf7615b3f927344fd77ea49b9f5dcb.1540493630.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/3] http: add support for selecting SSL backends at runtime","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-12-13T09:33:41Z","receivedAt":"2018-12-13T09:33:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Oct 25 2018, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Johannes Schindelin \n>\n> As of version 7.56.0, curl supports being compiled with multiple SSL\n> backends.\n>\n> This patch adds the Git side of that feature: by setting http.sslBackend\n> to \"openssl\" or \"schannel\", Git for Windows can now choose the SSL\n> backend at runtime.\n>\n> This comes in handy on Windows because Secure Channel (\"schannel\") is\n> the native solution, accessing the Windows Credential Store, thereby\n> allowing for enterprise-wide management of certificates. For historical\n> reasons, Git for Windows needs to support OpenSSL still, as it has\n> previously been the only supported SSL backend in Git for Windows for\n> almost a decade.\n>\n> The patch has been carried in Git for Windows for over a year, and is\n> considered mature.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> Documentation/config.txt | 5 +++++\n> http.c | 35 +++++++++++++++++++++++++++++++++++\n> 2 files changed, 40 insertions(+)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 154683321..7d38f0bf1 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1984,6 +1984,11 @@ http.sslCAPath::\n> \twith when fetching or pushing over HTTPS. Can be overridden\n> \tby the `GIT_SSL_CAPATH` environment variable.\n>\n> +http.sslBackend::\n> +\tName of the SSL backend to use (e.g. \"openssl\" or \"schannel\").\n> +\tThis option is ignored if cURL lacks support for choosing the SSL\n> +\tbackend at runtime.\n> +\n> http.pinnedpubkey::\n> \tPublic key of the https service. It may either be the filename of\n> \ta PEM or DER encoded public key file or a string starting with\n> diff --git a/http.c b/http.c\n> index 98ff12258..7fb37a061 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -155,6 +155,8 @@ static struct active_request_slot *active_queue_head;\n>\n> static char *cached_accept_language;\n>\n> +static char *http_ssl_backend;\n> +\n> size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)\n> {\n> \tsize_t size = eltsize * nmemb;\n> @@ -302,6 +304,12 @@ static int http_options(const char *var, const char *value, void *cb)\n> \t\tcurl_ssl_try = git_config_bool(var, value);\n> \t\treturn 0;\n> \t}\n> +\tif (!strcmp(\"http.sslbackend\", var)) {\n> +\t\tfree(http_ssl_backend);\n> +\t\thttp_ssl_backend = xstrdup_or_null(value);\n> +\t\treturn 0;\n> +\t}\n> +\n> \tif (!strcmp(\"http.minsessions\", var)) {\n> \t\tmin_curl_sessions = git_config_int(var, value);\n> #ifndef USE_CURL_MULTI\n> @@ -995,6 +1003,33 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)\n> \tgit_config(urlmatch_config_entry, &config);\n> \tfree(normalized_url);\n>\n> +#if LIBCURL_VERSION_NUM >= 0x073800\n> +\tif (http_ssl_backend) {\n> +\t\tconst curl_ssl_backend **backends;\n> +\t\tstruct strbuf buf = STRBUF_INIT;\n> +\t\tint i;\n> +\n> +\t\tswitch (curl_global_sslset(-1, http_ssl_backend, &backends)) {\n> +\t\tcase CURLSSLSET_UNKNOWN_BACKEND:\n> +\t\t\tstrbuf_addf(&buf, _(\"Unsupported SSL backend '%s'. \"\n> +\t\t\t\t\t \"Supported SSL backends:\"),\n> +\t\t\t\t\t http_ssl_backend);\n> +\t\t\tfor (i = 0; backends[i]; i++)\n> +\t\t\t\tstrbuf_addf(&buf, \"\\n\\t%s\", backends[i]->name);\n> +\t\t\tdie(\"%s\", buf.buf);\n> +\t\tcase CURLSSLSET_NO_BACKENDS:\n> +\t\t\tdie(_(\"Could not set SSL backend to '%s': \"\n> +\t\t\t \"cURL was built without SSL backends\"),\n> +\t\t\t http_ssl_backend);\n> +\t\tcase CURLSSLSET_TOO_LATE:\n> +\t\t\tdie(_(\"Could not set SSL backend to '%s': already set\"),\n> +\t\t\t http_ssl_backend);\n> +\t\tcase CURLSSLSET_OK:\n> +\t\t\tbreak; /* Okay! */\n> +\t\t}\n> +\t}\n> +#endif\n> +\n> \tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n> \t\tdie(\"curl_global_init failed\");\n\nHere's someone who upgraded to 2.20 on Arch linux & started getting\n\"Could not set...\" errors because of this change:\nhttps://www.reddit.com/r/git/comments/a5ne5v/git_fatal_could_not_set_ssl_backend_to_openssl/\n\nI don't know the context well enough, but is there perhaps enough info\nhere so we could give a better error message, e.g. \"don't set xyz twice\nin your config\", or just emit a warning?\n"},{"id":"365245","messageId":"nycvar.QRO.7.76.6.1812131403310.43@tvgsbejvaqbjf.bet","threadId":"49568","inReplyTo":"87sgz1n53e.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2 1/3] http: add support for selecting SSL backends at runtime","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-12-13T13:08:28Z","receivedAt":"2018-12-13T13:08:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ævar,\n\nOn Thu, 13 Dec 2018, Ævar Arnfjörð Bjarmason wrote:\n\n> On Thu, Oct 25 2018, Johannes Schindelin via GitGitGadget wrote:\n> \n> > From: Johannes Schindelin \n> >\n> > As of version 7.56.0, curl supports being compiled with multiple SSL\n> > backends.\n> >\n> > This patch adds the Git side of that feature: by setting http.sslBackend\n> > to \"openssl\" or \"schannel\", Git for Windows can now choose the SSL\n> > backend at runtime.\n> >\n> > This comes in handy on Windows because Secure Channel (\"schannel\") is\n> > the native solution, accessing the Windows Credential Store, thereby\n> > allowing for enterprise-wide management of certificates. For historical\n> > reasons, Git for Windows needs to support OpenSSL still, as it has\n> > previously been the only supported SSL backend in Git for Windows for\n> > almost a decade.\n> >\n> > The patch has been carried in Git for Windows for over a year, and is\n> > considered mature.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > Documentation/config.txt | 5 +++++\n> > http.c | 35 +++++++++++++++++++++++++++++++++++\n> > 2 files changed, 40 insertions(+)\n> >\n> > diff --git a/Documentation/config.txt b/Documentation/config.txt\n> > index 154683321..7d38f0bf1 100644\n> > --- a/Documentation/config.txt\n> > +++ b/Documentation/config.txt\n> > @@ -1984,6 +1984,11 @@ http.sslCAPath::\n> > \twith when fetching or pushing over HTTPS. Can be overridden\n> > \tby the `GIT_SSL_CAPATH` environment variable.\n> >\n> > +http.sslBackend::\n> > +\tName of the SSL backend to use (e.g. \"openssl\" or \"schannel\").\n> > +\tThis option is ignored if cURL lacks support for choosing the SSL\n> > +\tbackend at runtime.\n> > +\n> > http.pinnedpubkey::\n> > \tPublic key of the https service. It may either be the filename of\n> > \ta PEM or DER encoded public key file or a string starting with\n> > diff --git a/http.c b/http.c\n> > index 98ff12258..7fb37a061 100644\n> > --- a/http.c\n> > +++ b/http.c\n> > @@ -155,6 +155,8 @@ static struct active_request_slot *active_queue_head;\n> >\n> > static char *cached_accept_language;\n> >\n> > +static char *http_ssl_backend;\n> > +\n> > size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)\n> > {\n> > \tsize_t size = eltsize * nmemb;\n> > @@ -302,6 +304,12 @@ static int http_options(const char *var, const char *value, void *cb)\n> > \t\tcurl_ssl_try = git_config_bool(var, value);\n> > \t\treturn 0;\n> > \t}\n> > +\tif (!strcmp(\"http.sslbackend\", var)) {\n> > +\t\tfree(http_ssl_backend);\n> > +\t\thttp_ssl_backend = xstrdup_or_null(value);\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > \tif (!strcmp(\"http.minsessions\", var)) {\n> > \t\tmin_curl_sessions = git_config_int(var, value);\n> > #ifndef USE_CURL_MULTI\n> > @@ -995,6 +1003,33 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)\n> > \tgit_config(urlmatch_config_entry, &config);\n> > \tfree(normalized_url);\n> >\n> > +#if LIBCURL_VERSION_NUM >= 0x073800\n> > +\tif (http_ssl_backend) {\n> > +\t\tconst curl_ssl_backend **backends;\n> > +\t\tstruct strbuf buf = STRBUF_INIT;\n> > +\t\tint i;\n> > +\n> > +\t\tswitch (curl_global_sslset(-1, http_ssl_backend, &backends)) {\n> > +\t\tcase CURLSSLSET_UNKNOWN_BACKEND:\n> > +\t\t\tstrbuf_addf(&buf, _(\"Unsupported SSL backend '%s'. \"\n> > +\t\t\t\t\t \"Supported SSL backends:\"),\n> > +\t\t\t\t\t http_ssl_backend);\n> > +\t\t\tfor (i = 0; backends[i]; i++)\n> > +\t\t\t\tstrbuf_addf(&buf, \"\\n\\t%s\", backends[i]->name);\n> > +\t\t\tdie(\"%s\", buf.buf);\n> > +\t\tcase CURLSSLSET_NO_BACKENDS:\n> > +\t\t\tdie(_(\"Could not set SSL backend to '%s': \"\n> > +\t\t\t \"cURL was built without SSL backends\"),\n> > +\t\t\t http_ssl_backend);\n> > +\t\tcase CURLSSLSET_TOO_LATE:\n> > +\t\t\tdie(_(\"Could not set SSL backend to '%s': already set\"),\n> > +\t\t\t http_ssl_backend);\n> > +\t\tcase CURLSSLSET_OK:\n> > +\t\t\tbreak; /* Okay! */\n> > +\t\t}\n> > +\t}\n> > +#endif\n> > +\n> > \tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n> > \t\tdie(\"curl_global_init failed\");\n> \n> Here's someone who upgraded to 2.20 on Arch linux & started getting\n> \"Could not set...\" errors because of this change:\n> https://www.reddit.com/r/git/comments/a5ne5v/git_fatal_could_not_set_ssl_backend_to_openssl/\n\nYeah, I don't see bug reports that were opened via Reddit.\n\n> I don't know the context well enough, but is there perhaps enough info\n> here so we could give a better error message, e.g. \"don't set xyz twice\n> in your config\", or just emit a warning?\n\nThis is actually not the symptom of a Git bug, but of a cURL bug that I\nfixed in https://github.com/curl/curl/pull/3346. I suspect the fix for\nthis particular symptom to be\nhttps://github.com/curl/curl/commit/231a328c1c563acb53d8222894975e96bf7e6ea7\n\n(Granted, I introduced that bug, and did not catch it earlier because I\nalmost never build cURL with a single TLS backend these days, and that is\nnecessary to trigger the bug.)\n\nAccording to https://curl.haxx.se/changes.html, this bug fix\n(https://curl.haxx.se/bug/?i=3346) made it into v7.63.0, which is one day\nold.\n\nFeel free to update that Reddit post (I don't have an account, nor any\ninclination to get one).\n\nCiao,\nDscho"},{"id":"365246","messageId":"nycvar.QRO.7.76.6.1812131411030.43@tvgsbejvaqbjf.bet","threadId":"49568","inReplyTo":"nycvar.QRO.7.76.6.1812131403310.43@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 1/3] http: add support for selecting SSL backends at runtime","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-12-13T13:15:42Z","receivedAt":"2018-12-13T13:16:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 13 Dec 2018, Johannes Schindelin wrote:\n\n> On Thu, 13 Dec 2018, Ævar Arnfjörð Bjarmason wrote:\n> \n> > On Thu, Oct 25 2018, Johannes Schindelin via GitGitGadget wrote:\n> > \n> > > From: Johannes Schindelin \n> > >\n> > > As of version 7.56.0, curl supports being compiled with multiple SSL\n> > > backends.\n> > >\n> > > This patch adds the Git side of that feature: by setting http.sslBackend\n> > > to \"openssl\" or \"schannel\", Git for Windows can now choose the SSL\n> > > backend at runtime.\n> > >\n> > > This comes in handy on Windows because Secure Channel (\"schannel\") is\n> > > the native solution, accessing the Windows Credential Store, thereby\n> > > allowing for enterprise-wide management of certificates. For historical\n> > > reasons, Git for Windows needs to support OpenSSL still, as it has\n> > > previously been the only supported SSL backend in Git for Windows for\n> > > almost a decade.\n> > >\n> > > The patch has been carried in Git for Windows for over a year, and is\n> > > considered mature.\n> > >\n> > > Signed-off-by: Johannes Schindelin \n> > > ---\n> > > Documentation/config.txt | 5 +++++\n> > > http.c | 35 +++++++++++++++++++++++++++++++++++\n> > > 2 files changed, 40 insertions(+)\n> > >\n> > > diff --git a/Documentation/config.txt b/Documentation/config.txt\n> > > index 154683321..7d38f0bf1 100644\n> > > --- a/Documentation/config.txt\n> > > +++ b/Documentation/config.txt\n> > > @@ -1984,6 +1984,11 @@ http.sslCAPath::\n> > > \twith when fetching or pushing over HTTPS. Can be overridden\n> > > \tby the `GIT_SSL_CAPATH` environment variable.\n> > >\n> > > +http.sslBackend::\n> > > +\tName of the SSL backend to use (e.g. \"openssl\" or \"schannel\").\n> > > +\tThis option is ignored if cURL lacks support for choosing the SSL\n> > > +\tbackend at runtime.\n> > > +\n> > > http.pinnedpubkey::\n> > > \tPublic key of the https service. It may either be the filename of\n> > > \ta PEM or DER encoded public key file or a string starting with\n> > > diff --git a/http.c b/http.c\n> > > index 98ff12258..7fb37a061 100644\n> > > --- a/http.c\n> > > +++ b/http.c\n> > > @@ -155,6 +155,8 @@ static struct active_request_slot *active_queue_head;\n> > >\n> > > static char *cached_accept_language;\n> > >\n> > > +static char *http_ssl_backend;\n> > > +\n> > > size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)\n> > > {\n> > > \tsize_t size = eltsize * nmemb;\n> > > @@ -302,6 +304,12 @@ static int http_options(const char *var, const char *value, void *cb)\n> > > \t\tcurl_ssl_try = git_config_bool(var, value);\n> > > \t\treturn 0;\n> > > \t}\n> > > +\tif (!strcmp(\"http.sslbackend\", var)) {\n> > > +\t\tfree(http_ssl_backend);\n> > > +\t\thttp_ssl_backend = xstrdup_or_null(value);\n> > > +\t\treturn 0;\n> > > +\t}\n> > > +\n> > > \tif (!strcmp(\"http.minsessions\", var)) {\n> > > \t\tmin_curl_sessions = git_config_int(var, value);\n> > > #ifndef USE_CURL_MULTI\n> > > @@ -995,6 +1003,33 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)\n> > > \tgit_config(urlmatch_config_entry, &config);\n> > > \tfree(normalized_url);\n> > >\n> > > +#if LIBCURL_VERSION_NUM >= 0x073800\n> > > +\tif (http_ssl_backend) {\n> > > +\t\tconst curl_ssl_backend **backends;\n> > > +\t\tstruct strbuf buf = STRBUF_INIT;\n> > > +\t\tint i;\n> > > +\n> > > +\t\tswitch (curl_global_sslset(-1, http_ssl_backend, &backends)) {\n> > > +\t\tcase CURLSSLSET_UNKNOWN_BACKEND:\n> > > +\t\t\tstrbuf_addf(&buf, _(\"Unsupported SSL backend '%s'. \"\n> > > +\t\t\t\t\t \"Supported SSL backends:\"),\n> > > +\t\t\t\t\t http_ssl_backend);\n> > > +\t\t\tfor (i = 0; backends[i]; i++)\n> > > +\t\t\t\tstrbuf_addf(&buf, \"\\n\\t%s\", backends[i]->name);\n> > > +\t\t\tdie(\"%s\", buf.buf);\n> > > +\t\tcase CURLSSLSET_NO_BACKENDS:\n> > > +\t\t\tdie(_(\"Could not set SSL backend to '%s': \"\n> > > +\t\t\t \"cURL was built without SSL backends\"),\n> > > +\t\t\t http_ssl_backend);\n> > > +\t\tcase CURLSSLSET_TOO_LATE:\n> > > +\t\t\tdie(_(\"Could not set SSL backend to '%s': already set\"),\n> > > +\t\t\t http_ssl_backend);\n> > > +\t\tcase CURLSSLSET_OK:\n> > > +\t\t\tbreak; /* Okay! */\n> > > +\t\t}\n> > > +\t}\n> > > +#endif\n> > > +\n> > > \tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n> > > \t\tdie(\"curl_global_init failed\");\n> > \n> > Here's someone who upgraded to 2.20 on Arch linux & started getting\n> > \"Could not set...\" errors because of this change:\n> > https://www.reddit.com/r/git/comments/a5ne5v/git_fatal_could_not_set_ssl_backend_to_openssl/\n> \n> Yeah, I don't see bug reports that were opened via Reddit.\n> \n> > I don't know the context well enough, but is there perhaps enough info\n> > here so we could give a better error message, e.g. \"don't set xyz twice\n> > in your config\", or just emit a warning?\n> \n> This is actually not the symptom of a Git bug, but of a cURL bug that I\n> fixed in https://github.com/curl/curl/pull/3346. I suspect the fix for\n> this particular symptom to be\n> https://github.com/curl/curl/commit/231a328c1c563acb53d8222894975e96bf7e6ea7\n\nI should actually talk about that symptom a bit more so you understand\nwhere it comes from: the idea of cURL when compiled with multiple TLS\nbackends is that it has a meta backend that, upon first call, will see\nwhich backend to use and then plugs that.\n\nWhen compiled with a single backend, that meta backend is not plugged at\nall, the single backend is plugged by default.\n\nAnd here lies the rub, when *now* trying to select a TLS backend *by\nname*, the code incorrectly reported an error (instead of success in case\nthat the correct backend was already \"selected\", or *another* failure if\ntrying to select a backend that was not compiled in).\n\nCiao,\nDscho\n\n> (Granted, I introduced that bug, and did not catch it earlier because I\n> almost never build cURL with a single TLS backend these days, and that is\n> necessary to trigger the bug.)\n> \n> According to https://curl.haxx.se/changes.html, this bug fix\n> (https://curl.haxx.se/bug/?i=3346) made it into v7.63.0, which is one day\n> old.\n> \n> Feel free to update that Reddit post (I don't have an account, nor any\n> inclination to get one).\n> \n> Ciao,\n> Dscho"}]}