Re: [RFC PATCH v2] http: support CURLPROXY_HTTPS

Ævar Arnfjörð Bjarmason [off-list ref] writes:

On Sat, Dec 23 2017, Wei Shuyu jotted:

quoted

Git has been taught to support an https:// used for http.proxy when
using recent versions of libcurl.

To use https proxy with self-signed certs, we need a way to
unset CURLOPT_PROXY_SSL_VERIFYPEER and CURLOPT_PROXY_SSL_VERIFYHOST
just like direct SSL connection. This is required if we want
use t/lib-httpd to test proxy.

In this patch I reuse http.sslverify to do this, do we need an
independent option like http.sslproxyverify?

To fully support https proxy, we also need ways to set more options
such as CURLOPT_PROXY_SSLCERT. However, I'm not sure if we need to
support them.

It would be good to add a link to
https://daniel.haxx.se/blog/2016/11/26/https-proxy-with-curl/ to the
commit message, since it explains in great detail what this is for and
how it compares to what we were doing before.

Yeah, I found it a quite readable explanation.

It appears to me that the configuration for the proxy would have to
be different from the configuration for the hosts in the outside
world.  You may want to be a lot more strongly suspicious to the
latter while trusting the former a bit more.  I also suspect that
we'd need options like PROXY_SSLCERT that are parallel to the main
SSL support in the final form, but it may be OK to leave it to a
follow-up series.

And in order to start small and grow later, I think it is not a good
idea to repurpose http.sslverify to imply http.sslproxyverify; you
are declaring "when you disable peer verification, you also disable
proxy verification", and need to worry about backward compatibility
when you later introduce http.sslproxyverify that can control these
independently.  It probably is less bad than usual b/c issues in
that it probably is much more common to disable verification for
proxy than target hosts, though.

In any case, thanks for working on this.

quoted

Signed-off-by: Wei Shuyu <redacted>
---
 http.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/http.c b/http.c
index 215bebef1..d8a5e48f0 100644
--- a/http.c
+++ b/http.c

@@ -708,6 +708,10 @@ static CURL *get_curl_handle(void)
 	if (!curl_ssl_verify) {
 		curl_easy_setopt(result, CURLOPT_SSL_VERIFYPEER, 0);
 		curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 0);
+#if LIBCURL_VERSION_NUM >= 0x073400
+		curl_easy_setopt(result, CURLOPT_PROXY_SSL_VERIFYPEER, 0);
+		curl_easy_setopt(result, CURLOPT_PROXY_SSL_VERIFYHOST, 0);
+#endif
 	} else {
 		/* Verify authenticity of the peer's certificate */
 		curl_easy_setopt(result, CURLOPT_SSL_VERIFYPEER, 1);

@@ -865,6 +869,11 @@ static CURL *get_curl_handle(void)
 		else if (starts_with(curl_http_proxy, "socks"))
 			curl_easy_setopt(result,
 				CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4);
+#endif
+#if LIBCURL_VERSION_NUM >= 0x073400
+		else if (starts_with(curl_http_proxy, "https"))
+			curl_easy_setopt(result,
+				CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);
 #endif
 		if (strstr(curl_http_proxy, "://"))
 			credential_from_url(&proxy_auth, curl_http_proxy);