threads / patch / 52865

patch, 2 partsAdd HTTPS proxy SSL options (cert, key, cainfo)

Subject: [PATCH 0/2] Add HTTPS proxy SSL options (cert, key, cainfo)

## tl;dr

15 messages between Feb 21, 2020 and Mar 4, 2020. Diffs are folded; open one to read it.

replies: 14people: 5as markdown or json

Jorge via GitGitGadget· Feb 21, 2020, 21:36 UTC · lore

Git currently supports connecting to proxies through HTTPS. However it does not allow you to configure SSL options when connecting (i.e. client cert, key, cainfo). These set of commits add the necessary options and documentation needed to support them.

Libcurl already has support for this so changes are somewhat minimal.

I didn't see integration tests under /t or tests that verified libcurl integration. Is there another recommended way to add unit tests for these changes? I did verify manually with an HTTPS proxy that the options were having the desired effect.

./bin-wrappers/git -c http.proxy=https://<PROXY-HOSTNAME> \ -c http.proxycert=<CERT> -c http.proxykey=<KEY> \ clone https://github.com/jalopezsilva/dotfiles.git

Jorge Lopez Silva (2):
  http: add client cert for HTTPS proxies.
  config: documentation for HTTPS proxy client cert.
 Documentation/config/http.txt | 14 ++++++++++
 http.c                        | 48 ++++++++++++++++++++++++++++++++---
 2 files changed, 59 insertions(+), 3 deletions(-)
base-commit: 51ebf55b9309824346a6589c9f3b130c6f371b8f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-559%2Fjalopezsilva%2Fhttps_proxy_ssl_options-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-559/jalopezsilva/https_proxy_ssl_options-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/559
-- 
gitgitgadget
Jorge Lopez Silva via GitGitGadget· Feb 21, 2020, 21:36 UTC · re: Jorge via GitGitGadget · lore

[PATCH 1/2] http: add client cert for HTTPS proxies.

From: Jorge Lopez Silva <jalopezsilva@gmail.com>

Git currently supports performing connections to HTTPS proxies but we don't support doing mutual authentication with them (through TLS). This commit adds the necessary options to be able to send a client certificate to the HTTPS proxy.

A client certificate can provide an alternative way of authentication instead of using 'ProxyAuthorization' or other more common methods of authentication.

Libcurl supports this functionality already. The feature is guarded by the first available libcurl version that supports these options.

Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com>
---
 http.c | 48 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 45 insertions(+), 3 deletions(-)
Show changes to http.c +45 −3
diff --git a/http.c b/http.c
index 00a0e507633..141cf8f80cd 100644
--- a/http.c
+++ b/http.c
@@ -86,6 +86,14 @@ static long curl_low_speed_time = -1;
 static int curl_ftp_no_epsv;
 static const char *curl_http_proxy;
 static const char *http_proxy_authmethod;
+
+#if LIBCURL_VERSION_NUM >= 0x073400
+static const char *http_proxy_ssl_cert;
+static const char *http_proxy_ssl_key;
+static const char *http_proxy_ssl_key_passwd;
+#endif
+static const char *http_proxy_ssl_ca_info;
+
 static struct {
 	const char *name;
 	long curlauth_param;
@@ -365,6 +373,20 @@ static int http_options(const char *var, const char *value, void *cb)
 	if (!strcmp("http.proxyauthmethod", var))
 		return git_config_string(&http_proxy_authmethod, var, value);
 
+#if LIBCURL_VERSION_NUM >= 0x073400
+	if (!strcmp("http.proxycert", var))
+		return git_config_string(&http_proxy_ssl_cert, var, value);
+
+	if (!strcmp("http.proxykey", var))
+		return git_config_string(&http_proxy_ssl_key, var, value);
+
+	if (!strcmp("http.proxykeypass", var))
+		return git_config_string(&http_proxy_ssl_key_passwd, var, value);
+
+	if (!strcmp("http.proxycainfo", var))
+		return git_config_string(&http_proxy_ssl_ca_info, var, value);
+#endif
+
 	if (!strcmp("http.cookiefile", var))
 		return git_config_pathname(&curl_cookie_file, var, value);
 	if (!strcmp("http.savecookies", var)) {
@@ -924,8 +946,14 @@ static CURL *get_curl_handle(void)
 #if LIBCURL_VERSION_NUM >= 0x073400
 		curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, NULL);
 #endif
-	} else if (ssl_cainfo != NULL)
-		curl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);
+	} else if (ssl_cainfo != NULL || http_proxy_ssl_ca_info != NULL) {
+		if (ssl_cainfo != NULL)
+			curl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);
+#if LIBCURL_VERSION_NUM >= 0x073400
+		if (http_proxy_ssl_ca_info != NULL)
+			curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, http_proxy_ssl_ca_info);
+#endif
+	}
 
 	if (curl_low_speed_limit > 0 && curl_low_speed_time > 0) {
 		curl_easy_setopt(result, CURLOPT_LOW_SPEED_LIMIT,
@@ -1018,9 +1046,23 @@ static CURL *get_curl_handle(void)
 				CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4);
 #endif
 #if LIBCURL_VERSION_NUM >= 0x073400
-		else if (starts_with(curl_http_proxy, "https"))
+		else if (starts_with(curl_http_proxy, "https")) {
 			curl_easy_setopt(result,
 				CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);
+
+			if (http_proxy_ssl_cert != NULL) {
+				curl_easy_setopt(result,
+					CURLOPT_PROXY_SSLCERT, http_proxy_ssl_cert);
+				}
+			if (http_proxy_ssl_key != NULL) {
+				curl_easy_setopt(result,
+					CURLOPT_PROXY_SSLKEY, http_proxy_ssl_key);
+				}
+			if (http_proxy_ssl_key_passwd != NULL) {
+				curl_easy_setopt(result,
+					CURLOPT_PROXY_KEYPASSWD, http_proxy_ssl_key_passwd);
+				}
+			}
 #endif
 		if (strstr(curl_http_proxy, "://"))
 			credential_from_url(&proxy_auth, curl_http_proxy);
-- 
gitgitgadget
Eric Sunshine· Feb 21, 2020, 22:28 UTC · re: Jorge Lopez Silva via GitGitGadget · lore

Re: [PATCH 1/2] http: add client cert for HTTPS proxies.

On Fri, Feb 21, 2020 at 4:37 PM Jorge Lopez Silva via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 29 quoted lines
> Git currently supports performing connections to HTTPS proxies but we
> don't support doing mutual authentication with them (through TLS). This
> commit adds the necessary options to be able to send a client
> certificate to the HTTPS proxy.
> [...]
> Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com>
> ---
> diff --git a/http.c b/http.c
> @@ -1018,9 +1046,23 @@ static CURL *get_curl_handle(void)
>  #if LIBCURL_VERSION_NUM >= 0x073400
> -               else if (starts_with(curl_http_proxy, "https"))
> +               else if (starts_with(curl_http_proxy, "https")) {
>                         curl_easy_setopt(result,
>                                 CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);
> +
> +                       if (http_proxy_ssl_cert != NULL) {
> +                               curl_easy_setopt(result,
> +                                       CURLOPT_PROXY_SSLCERT, http_proxy_ssl_cert);
> +                               }
> +                       if (http_proxy_ssl_key != NULL) {
> +                               curl_easy_setopt(result,
> +                                       CURLOPT_PROXY_SSLKEY, http_proxy_ssl_key);
> +                               }
> +                       if (http_proxy_ssl_key_passwd != NULL) {
> +                               curl_easy_setopt(result,
> +                                       CURLOPT_PROXY_KEYPASSWD, http_proxy_ssl_key_passwd);
> +                               }
> +                       }
>  #endif

All the closing braces in this hunk seem to be over-indented. Also, all of the braces for the one-liner 'if' bodies can be dropped, thus making it less noisy.

Jorge A López Silva· Feb 26, 2020, 21:05 UTC · re: Eric Sunshine · lore

Re: [PATCH 1/2] http: add client cert for HTTPS proxies.

Thanks Eric for the feedback. I'm addressing your comments and sending a v2.
On Fri, Feb 21, 2020 at 2:28 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 36 quoted lines
>
> On Fri, Feb 21, 2020 at 4:37 PM Jorge Lopez Silva via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> > Git currently supports performing connections to HTTPS proxies but we
> > don't support doing mutual authentication with them (through TLS). This
> > commit adds the necessary options to be able to send a client
> > certificate to the HTTPS proxy.
> > [...]
> > Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com>
> > ---
> > diff --git a/http.c b/http.c
> > @@ -1018,9 +1046,23 @@ static CURL *get_curl_handle(void)
> >  #if LIBCURL_VERSION_NUM >= 0x073400
> > -               else if (starts_with(curl_http_proxy, "https"))
> > +               else if (starts_with(curl_http_proxy, "https")) {
> >                         curl_easy_setopt(result,
> >                                 CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);
> > +
> > +                       if (http_proxy_ssl_cert != NULL) {
> > +                               curl_easy_setopt(result,
> > +                                       CURLOPT_PROXY_SSLCERT, http_proxy_ssl_cert);
> > +                               }
> > +                       if (http_proxy_ssl_key != NULL) {
> > +                               curl_easy_setopt(result,
> > +                                       CURLOPT_PROXY_SSLKEY, http_proxy_ssl_key);
> > +                               }
> > +                       if (http_proxy_ssl_key_passwd != NULL) {
> > +                               curl_easy_setopt(result,
> > +                                       CURLOPT_PROXY_KEYPASSWD, http_proxy_ssl_key_passwd);
> > +                               }
> > +                       }
> >  #endif
>
> All the closing braces in this hunk seem to be over-indented. Also,
> all of the braces for the one-liner 'if' bodies can be dropped, thus
> making it less noisy.
Jorge Lopez Silva via GitGitGadget· Feb 21, 2020, 21:36 UTC · re: Jorge via GitGitGadget · lore

[PATCH 2/2] config: documentation for HTTPS proxy client cert.

From: Jorge Lopez Silva <jalopezsilva@gmail.com>

The commit adds 4 options, client cert, key, key password and CA info. The CA info can be used to specify a different CA path to validate the HTTPS proxy cert.

Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com>
---
 Documentation/config/http.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
Show changes to Documentation/config/http.txt +14 −0
diff --git a/Documentation/config/http.txt b/Documentation/config/http.txt
index e806033aab8..7e704687e87 100644
--- a/Documentation/config/http.txt
+++ b/Documentation/config/http.txt
@@ -29,6 +29,20 @@ http.proxyAuthMethod::
 * `ntlm` - NTLM authentication (compare the --ntlm option of `curl(1)`)
 --
 
+http.proxycert::
+	File indicating a client certificate to use to authenticate with an HTTPS proxy.
+
+http.proxykey::
+	File indicating a private key to use to authenticate with an HTTPS proxy.
+
+http.proxykeypass::
+	When communicating to the proxy using TLS (using an HTTPS proxy), use this
+	option along `http.proxykey` to indicate a password for the key.
+
+http.proxycainfo::
+	File containing the certificates to verify the proxy with when using an HTTPS
+	proxy.
+
 http.emptyAuth::
 	Attempt authentication without seeking a username or password.  This
 	can be used to attempt GSS-Negotiate authentication without specifying
-- 
gitgitgadget
Jorge via GitGitGadget· Feb 26, 2020, 23:23 UTC · re: Jorge via GitGitGadget · lore

[PATCH v2 0/2] Add HTTPS proxy SSL options (cert, key, cainfo)

Git currently supports connecting to proxies through HTTPS. However it does not allow you to configure SSL options when connecting (i.e. client cert, key, cainfo). These set of commits add the necessary options and documentation needed to support them.

Libcurl already has support for this so changes are somewhat minimal.

I ran the CI tests and verified manually with an HTTPS proxy that changes are working as expected. I didn't see integration tests under /t or tests that verified libcurl integration.

./bin-wrappers/git -c http.proxy=https://<PROXY-HOSTNAME> \ -c http.proxycert=<CERT> -c http.proxykey=<KEY> \ clone https://github.com/jalopezsilva/dotfiles.git

Jorge Lopez Silva (2):
  http: add client cert for HTTPS proxies.
  config: documentation for HTTPS proxy client cert.
 Documentation/config/http.txt | 14 ++++++++++
 http.c                        | 48 +++++++++++++++++++++++++++++++----
 2 files changed, 57 insertions(+), 5 deletions(-)
base-commit: 51ebf55b9309824346a6589c9f3b130c6f371b8f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-559%2Fjalopezsilva%2Fhttps_proxy_ssl_options-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-559/jalopezsilva/https_proxy_ssl_options-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/559
Range-diff vs v1:
 1:  3cf866d0384 ! 1:  a5d980e7501 http: add client cert for HTTPS proxies.
     @@ -27,7 +27,7 @@
      +#if LIBCURL_VERSION_NUM >= 0x073400
      +static const char *http_proxy_ssl_cert;
      +static const char *http_proxy_ssl_key;
     -+static const char *http_proxy_ssl_key_passwd;
     ++static const char *http_proxy_ssl_keypasswd;
      +#endif
      +static const char *http_proxy_ssl_ca_info;
      +
     @@ -46,7 +46,7 @@
      +		return git_config_string(&http_proxy_ssl_key, var, value);
      +
      +	if (!strcmp("http.proxykeypass", var))
     -+		return git_config_string(&http_proxy_ssl_key_passwd, var, value);
     ++		return git_config_string(&http_proxy_ssl_keypasswd, var, value);
      +
      +	if (!strcmp("http.proxycainfo", var))
      +		return git_config_string(&http_proxy_ssl_ca_info, var, value);
     @@ -77,23 +77,21 @@
       #endif
       #if LIBCURL_VERSION_NUM >= 0x073400
      -		else if (starts_with(curl_http_proxy, "https"))
     +-			curl_easy_setopt(result,
     +-				CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);
      +		else if (starts_with(curl_http_proxy, "https")) {
     - 			curl_easy_setopt(result,
     - 				CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);
     ++			curl_easy_setopt(result, CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);
      +
     -+			if (http_proxy_ssl_cert != NULL) {
     -+				curl_easy_setopt(result,
     -+					CURLOPT_PROXY_SSLCERT, http_proxy_ssl_cert);
     -+				}
     -+			if (http_proxy_ssl_key != NULL) {
     -+				curl_easy_setopt(result,
     -+					CURLOPT_PROXY_SSLKEY, http_proxy_ssl_key);
     -+				}
     -+			if (http_proxy_ssl_key_passwd != NULL) {
     -+				curl_easy_setopt(result,
     -+					CURLOPT_PROXY_KEYPASSWD, http_proxy_ssl_key_passwd);
     -+				}
     -+			}
     ++			if (http_proxy_ssl_cert != NULL)
     ++				curl_easy_setopt(result, CURLOPT_PROXY_SSLCERT, http_proxy_ssl_cert);
     ++
     ++			if (http_proxy_ssl_key != NULL)
     ++				curl_easy_setopt(result, CURLOPT_PROXY_SSLKEY, http_proxy_ssl_key);
     ++
     ++			if (http_proxy_ssl_keypasswd != NULL)
     ++				curl_easy_setopt(result, CURLOPT_PROXY_KEYPASSWD, http_proxy_ssl_keypasswd);
     ++
     ++		}
       #endif
       		if (strstr(curl_http_proxy, "://"))
       			credential_from_url(&proxy_auth, curl_http_proxy);
 2:  583fdd0fe9b = 2:  c40207a3928 config: documentation for HTTPS proxy client cert.
-- 
gitgitgadget
Jorge Lopez Silva via GitGitGadget· Feb 26, 2020, 23:23 UTC · re: Jorge via GitGitGadget · lore

[PATCH v2 2/2] config: documentation for HTTPS proxy client cert.

From: Jorge Lopez Silva <jalopezsilva@gmail.com>

The commit adds 4 options, client cert, key, key password and CA info. The CA info can be used to specify a different CA path to validate the HTTPS proxy cert.

Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com>
---
 Documentation/config/http.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
Show changes to Documentation/config/http.txt +14 −0
diff --git a/Documentation/config/http.txt b/Documentation/config/http.txt
index e806033aab8..7e704687e87 100644
--- a/Documentation/config/http.txt
+++ b/Documentation/config/http.txt
@@ -29,6 +29,20 @@ http.proxyAuthMethod::
 * `ntlm` - NTLM authentication (compare the --ntlm option of `curl(1)`)
 --
 
+http.proxycert::
+	File indicating a client certificate to use to authenticate with an HTTPS proxy.
+
+http.proxykey::
+	File indicating a private key to use to authenticate with an HTTPS proxy.
+
+http.proxykeypass::
+	When communicating to the proxy using TLS (using an HTTPS proxy), use this
+	option along `http.proxykey` to indicate a password for the key.
+
+http.proxycainfo::
+	File containing the certificates to verify the proxy with when using an HTTPS
+	proxy.
+
 http.emptyAuth::
 	Attempt authentication without seeking a username or password.  This
 	can be used to attempt GSS-Negotiate authentication without specifying
-- 
gitgitgadget
Junio C Hamano· Feb 27, 2020, 18:58 UTC · re: Jorge Lopez Silva via GitGitGadget · lore

Re: [PATCH v2 2/2] config: documentation for HTTPS proxy client cert.

"Jorge Lopez Silva via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 8 quoted lines
> From: Jorge Lopez Silva <jalopezsilva@gmail.com>
>
> The commit adds 4 options, client cert, key, key password and CA info.
> The CA info can be used to specify a different CA path to validate the
> HTTPS proxy cert.
>
> Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com>
> ---

Thanks, this should be part of the previous patch, as it was that commit, not this one, that adds 4 options ;-)

Show 5 quoted lines
> +http.proxycert::
> +	File indicating a client certificate to use to authenticate with an HTTPS proxy.
> +
> +http.proxykey::
> +	File indicating a private key to use to authenticate with an HTTPS proxy.

I think these files not merely "indicate" but they themselves "hold", "contain" and/or "store" the certificate and key. Perhaps more like...

	The pathname of a file that stores a client certificate to ...

Also, it is customary to camelCase the configuration variable names. As I understand http.proxykey is roughly corresponds to existing http.sslKey (the former is for proxy, the latter is for the target host), I'd expect these two to be spelled http.proxySSLCert and http.proxySSLKey respectively (without omitting "SSL", as that is the underlying cURL option name if I am reading the code in 1/2 correctly).

> +http.proxykeypass::
> +	When communicating to the proxy using TLS (using an HTTPS proxy), use this
> +	option along `http.proxykey` to indicate a password for the key.
And this would be "http.proxyKeyPasswd" for the same two reasons.
There are a couple of curious things, though:
 * Is it a good idea to use a keyfile that is encrypted, but leave
   the encryption password on disk in the configuration file to
   begin with?
 * This teaches our system about PROXY_KEYPASSWD that protects
   PROXY_SSLKEY, but why isn't there a similar configuration
   variable for CURLOPT_KEYPASSWD that protects CURLOPT_SSLKEY?

It is possible that the answer to these questions are the same---an on-disk password is a bad idea, so we deliberately omit a config that gives value to CURLOPT_KEYPASSWD and instead use the credential subsystem (see http.c::has_cert_password() and its caller). If so, I think it would be prudent to follow the same pattern if possible?

Show 7 quoted lines
> +http.proxycainfo::
> +	File containing the certificates to verify the proxy with when using an HTTPS
> +	proxy.
> +
>  http.emptyAuth::
>  	Attempt authentication without seeking a username or password.  This
>  	can be used to attempt GSS-Negotiate authentication without specifying
Jorge A López Silva· Mar 3, 2020, 01:47 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 2/2] config: documentation for HTTPS proxy client cert.

> Thanks, this should be part of the previous patch, as it was that
> commit, not this one, that adds 4 options ;-)
Haha, yeah, you're right. I'll collapse the commits into a single one.
Show 11 quoted lines
>  I think these files not merely "indicate" but they themselves
> "hold", "contain" and/or "store" the certificate and key.  Perhaps
> more like...
>         The pathname of a file that stores a client certificate to ...
> Also, it is customary to camelCase the configuration variable names.
> As I understand http.proxykey is roughly corresponds to existing
> http.sslKey (the former is for proxy, the latter is for the target
> host), I'd expect these two to be spelled http.proxySSLCert and
> http.proxySSLKey respectively (without omitting "SSL", as that is
> the underlying cURL option name if I am reading the code in 1/2
> correctly).
Good point. Better descriptions and names will be added.
Show 5 quoted lines
> It is possible that the answer to these questions are the same---an
> on-disk password is a bad idea, so we deliberately omit a config
> that gives value to CURLOPT_KEYPASSWD and instead use the credential
> subsystem (see http.c::has_cert_password() and its caller).  If so,
> I think it would be prudent to follow the same pattern if possible?
Excellent point. Will adjust to re-use the same pattern.
On Thu, Feb 27, 2020 at 10:58 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 65 quoted lines
>
> "Jorge Lopez Silva via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Jorge Lopez Silva <jalopezsilva@gmail.com>
> >
> > The commit adds 4 options, client cert, key, key password and CA info.
> > The CA info can be used to specify a different CA path to validate the
> > HTTPS proxy cert.
> >
> > Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com>
> > ---
>
> Thanks, this should be part of the previous patch, as it was that
> commit, not this one, that adds 4 options ;-)
>
> > +http.proxycert::
> > +     File indicating a client certificate to use to authenticate with an HTTPS proxy.
> > +
> > +http.proxykey::
> > +     File indicating a private key to use to authenticate with an HTTPS proxy.
>
> I think these files not merely "indicate" but they themselves
> "hold", "contain" and/or "store" the certificate and key.  Perhaps
> more like...
>
>         The pathname of a file that stores a client certificate to ...
>
> Also, it is customary to camelCase the configuration variable names.
> As I understand http.proxykey is roughly corresponds to existing
> http.sslKey (the former is for proxy, the latter is for the target
> host), I'd expect these two to be spelled http.proxySSLCert and
> http.proxySSLKey respectively (without omitting "SSL", as that is
> the underlying cURL option name if I am reading the code in 1/2
> correctly).
>
> > +http.proxykeypass::
> > +     When communicating to the proxy using TLS (using an HTTPS proxy), use this
> > +     option along `http.proxykey` to indicate a password for the key.
>
> And this would be "http.proxyKeyPasswd" for the same two reasons.
>
> There are a couple of curious things, though:
>
>  * Is it a good idea to use a keyfile that is encrypted, but leave
>    the encryption password on disk in the configuration file to
>    begin with?
>
>  * This teaches our system about PROXY_KEYPASSWD that protects
>    PROXY_SSLKEY, but why isn't there a similar configuration
>    variable for CURLOPT_KEYPASSWD that protects CURLOPT_SSLKEY?
>
> It is possible that the answer to these questions are the same---an
> on-disk password is a bad idea, so we deliberately omit a config
> that gives value to CURLOPT_KEYPASSWD and instead use the credential
> subsystem (see http.c::has_cert_password() and its caller).  If so,
> I think it would be prudent to follow the same pattern if possible?
>
> > +http.proxycainfo::
> > +     File containing the certificates to verify the proxy with when using an HTTPS
> > +     proxy.
> > +
> >  http.emptyAuth::
> >       Attempt authentication without seeking a username or password.  This
> >       can be used to attempt GSS-Negotiate authentication without specifying
Jorge Lopez Silva via GitGitGadget· Feb 26, 2020, 23:23 UTC · re: Jorge via GitGitGadget · lore

[PATCH v2 1/2] http: add client cert for HTTPS proxies.

From: Jorge Lopez Silva <jalopezsilva@gmail.com>

Git currently supports performing connections to HTTPS proxies but we don't support doing mutual authentication with them (through TLS). This commit adds the necessary options to be able to send a client certificate to the HTTPS proxy.

A client certificate can provide an alternative way of authentication instead of using 'ProxyAuthorization' or other more common methods of authentication.

Libcurl supports this functionality already. The feature is guarded by the first available libcurl version that supports these options.

Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com>
---
 http.c | 48 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 5 deletions(-)
Show changes to http.c +43 −5
diff --git a/http.c b/http.c
index 00a0e507633..88782d39f15 100644
--- a/http.c
+++ b/http.c
@@ -86,6 +86,14 @@ static long curl_low_speed_time = -1;
 static int curl_ftp_no_epsv;
 static const char *curl_http_proxy;
 static const char *http_proxy_authmethod;
+
+#if LIBCURL_VERSION_NUM >= 0x073400
+static const char *http_proxy_ssl_cert;
+static const char *http_proxy_ssl_key;
+static const char *http_proxy_ssl_keypasswd;
+#endif
+static const char *http_proxy_ssl_ca_info;
+
 static struct {
 	const char *name;
 	long curlauth_param;
@@ -365,6 +373,20 @@ static int http_options(const char *var, const char *value, void *cb)
 	if (!strcmp("http.proxyauthmethod", var))
 		return git_config_string(&http_proxy_authmethod, var, value);
 
+#if LIBCURL_VERSION_NUM >= 0x073400
+	if (!strcmp("http.proxycert", var))
+		return git_config_string(&http_proxy_ssl_cert, var, value);
+
+	if (!strcmp("http.proxykey", var))
+		return git_config_string(&http_proxy_ssl_key, var, value);
+
+	if (!strcmp("http.proxykeypass", var))
+		return git_config_string(&http_proxy_ssl_keypasswd, var, value);
+
+	if (!strcmp("http.proxycainfo", var))
+		return git_config_string(&http_proxy_ssl_ca_info, var, value);
+#endif
+
 	if (!strcmp("http.cookiefile", var))
 		return git_config_pathname(&curl_cookie_file, var, value);
 	if (!strcmp("http.savecookies", var)) {
@@ -924,8 +946,14 @@ static CURL *get_curl_handle(void)
 #if LIBCURL_VERSION_NUM >= 0x073400
 		curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, NULL);
 #endif
-	} else if (ssl_cainfo != NULL)
-		curl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);
+	} else if (ssl_cainfo != NULL || http_proxy_ssl_ca_info != NULL) {
+		if (ssl_cainfo != NULL)
+			curl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);
+#if LIBCURL_VERSION_NUM >= 0x073400
+		if (http_proxy_ssl_ca_info != NULL)
+			curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, http_proxy_ssl_ca_info);
+#endif
+	}
 
 	if (curl_low_speed_limit > 0 && curl_low_speed_time > 0) {
 		curl_easy_setopt(result, CURLOPT_LOW_SPEED_LIMIT,
@@ -1018,9 +1046,19 @@ static CURL *get_curl_handle(void)
 				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);
+		else if (starts_with(curl_http_proxy, "https")) {
+			curl_easy_setopt(result, CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);
+
+			if (http_proxy_ssl_cert != NULL)
+				curl_easy_setopt(result, CURLOPT_PROXY_SSLCERT, http_proxy_ssl_cert);
+
+			if (http_proxy_ssl_key != NULL)
+				curl_easy_setopt(result, CURLOPT_PROXY_SSLKEY, http_proxy_ssl_key);
+
+			if (http_proxy_ssl_keypasswd != NULL)
+				curl_easy_setopt(result, CURLOPT_PROXY_KEYPASSWD, http_proxy_ssl_keypasswd);
+
+		}
 #endif
 		if (strstr(curl_http_proxy, "://"))
 			credential_from_url(&proxy_auth, curl_http_proxy);
-- 
gitgitgadget
Junio C Hamano· Feb 27, 2020, 18:31 UTC · re: Jorge Lopez Silva via GitGitGadget · lore

Re: [PATCH v2 1/2] http: add client cert for HTTPS proxies.

"Jorge Lopez Silva via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 27 quoted lines
> +#if LIBCURL_VERSION_NUM >= 0x073400
> +static const char *http_proxy_ssl_cert;
> +static const char *http_proxy_ssl_key;
> +static const char *http_proxy_ssl_keypasswd;
> +#endif
> +static const char *http_proxy_ssl_ca_info;
> +
>  static struct {
>  	const char *name;
>  	long curlauth_param;
> @@ -365,6 +373,20 @@ static int http_options(const char *var, const char *value, void *cb)
>  	if (!strcmp("http.proxyauthmethod", var))
>  		return git_config_string(&http_proxy_authmethod, var, value);
>  
> +#if LIBCURL_VERSION_NUM >= 0x073400
> +	if (!strcmp("http.proxycert", var))
> +		return git_config_string(&http_proxy_ssl_cert, var, value);
> +
> +	if (!strcmp("http.proxykey", var))
> +		return git_config_string(&http_proxy_ssl_key, var, value);
> +
> +	if (!strcmp("http.proxykeypass", var))
> +		return git_config_string(&http_proxy_ssl_keypasswd, var, value);
> +
> +	if (!strcmp("http.proxycainfo", var))
> +		return git_config_string(&http_proxy_ssl_ca_info, var, value);
> +#endif

You may copy around your ~/.gitconfig to multiple hosts, some may have newer and others may have older versions of libcurl, so it would be OK for a version of Git built with older libcurl to at least see and parse configurations meant for newer one, if only to ignore and discard.

The only two effects these #if/#endif have are (1) they save a tiny bit of memory, code and runtime cycle on an older platform and (2) they make the resuting code ugly and harder to read. I do not think that the tradeoff is worth it.

Show 17 quoted lines
>  	if (!strcmp("http.cookiefile", var))
>  		return git_config_pathname(&curl_cookie_file, var, value);
>  	if (!strcmp("http.savecookies", var)) {
> @@ -924,8 +946,14 @@ static CURL *get_curl_handle(void)
>  #if LIBCURL_VERSION_NUM >= 0x073400
>  		curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, NULL);
>  #endif
> -	} else if (ssl_cainfo != NULL)
> -		curl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);
> +	} else if (ssl_cainfo != NULL || http_proxy_ssl_ca_info != NULL) {
> +		if (ssl_cainfo != NULL)
> +			curl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);
> +#if LIBCURL_VERSION_NUM >= 0x073400
> +		if (http_proxy_ssl_ca_info != NULL)
> +			curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, http_proxy_ssl_ca_info);
> +#endif
> +	}

On this codepath, unlike the config and variable definitions, #if/#endif is absolutely necessary.

In any case, the code around here is messy, but it is mostly due to the fact that the existing #if/#endif with if/elseif/... cascade was messy. The general idea is

 * We want to honor ssl_cainfo and http_proxy_ssl_ca_info, and use
   CAINFO when set, but
 * When http_schannel_use_ssl_cainfo is not in effect and
   http_ssl_backend is schannel, ssl_cainfo/http_proxy_ssl_ca_info
   business is completely skipped, and these two CAINFO are cleared
   instead.

I do not know if the above is the best code structure to express that, but at least the way this patch adds code is the least noisy, I guess.

Show 18 quoted lines
> @@ -1018,9 +1046,19 @@ static CURL *get_curl_handle(void)
>  				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);
> +		else if (starts_with(curl_http_proxy, "https")) {
> +			curl_easy_setopt(result, CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);
> +
> +			if (http_proxy_ssl_cert != NULL)
> +				curl_easy_setopt(result, CURLOPT_PROXY_SSLCERT, http_proxy_ssl_cert);
> +
> +			if (http_proxy_ssl_key != NULL)
> +				curl_easy_setopt(result, CURLOPT_PROXY_SSLKEY, http_proxy_ssl_key);
> +
> +			if (http_proxy_ssl_keypasswd != NULL)
> +				curl_easy_setopt(result, CURLOPT_PROXY_KEYPASSWD, http_proxy_ssl_keypasswd);
This part is more or less straight-forward.

This is a minor tangent, but I see many "var != NULL" instances used as the condition to if statements, which we tend to frown upon (instead just say "if (var) ..."). I know there are already many in the existing code in this file, but this patch is making it even worse.

> +		}
>  #endif
>  		if (strstr(curl_http_proxy, "://"))
>  			credential_from_url(&proxy_auth, curl_http_proxy);
Jorge A López Silva· Mar 3, 2020, 01:41 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 1/2] http: add client cert for HTTPS proxies.

Show 9 quoted lines
> You may copy around your ~/.gitconfig to multiple hosts, some may
> have newer and others may have older versions of libcurl, so it
> would be OK for a version of Git built with older libcurl to at
> least see and parse configurations meant for newer one, if only
> to ignore and discard.
> The only two effects these #if/#endif have are (1) they save a tiny
> bit of memory, code and runtime cycle on an older platform and (2)
> they make the resuting code ugly and harder to read.  I do not think
> that the tradeoff is worth it.
I agree, thanks for the input. I'll remove the #if/#endif from the variables.
Show 6 quoted lines
>  This part is more or less straight-forward.
> This is a minor tangent, but I see many "var != NULL" instances used
> as the condition to if statements, which we tend to frown upon
> (instead just say "if (var) ...").  I know there are already many in
> the existing code in this file, but this patch is making it even
> worse.
Understood, will fix!
On Thu, Feb 27, 2020 at 10:31 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 111 quoted lines
>
> "Jorge Lopez Silva via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > +#if LIBCURL_VERSION_NUM >= 0x073400
> > +static const char *http_proxy_ssl_cert;
> > +static const char *http_proxy_ssl_key;
> > +static const char *http_proxy_ssl_keypasswd;
> > +#endif
> > +static const char *http_proxy_ssl_ca_info;
> > +
> >  static struct {
> >       const char *name;
> >       long curlauth_param;
> > @@ -365,6 +373,20 @@ static int http_options(const char *var, const char *value, void *cb)
> >       if (!strcmp("http.proxyauthmethod", var))
> >               return git_config_string(&http_proxy_authmethod, var, value);
> >
> > +#if LIBCURL_VERSION_NUM >= 0x073400
> > +     if (!strcmp("http.proxycert", var))
> > +             return git_config_string(&http_proxy_ssl_cert, var, value);
> > +
> > +     if (!strcmp("http.proxykey", var))
> > +             return git_config_string(&http_proxy_ssl_key, var, value);
> > +
> > +     if (!strcmp("http.proxykeypass", var))
> > +             return git_config_string(&http_proxy_ssl_keypasswd, var, value);
> > +
> > +     if (!strcmp("http.proxycainfo", var))
> > +             return git_config_string(&http_proxy_ssl_ca_info, var, value);
> > +#endif
>
> You may copy around your ~/.gitconfig to multiple hosts, some may
> have newer and others may have older versions of libcurl, so it
> would be OK for a version of Git built with older libcurl to at
> least see and parse configurations meant for newer one, if only
> to ignore and discard.
>
> The only two effects these #if/#endif have are (1) they save a tiny
> bit of memory, code and runtime cycle on an older platform and (2)
> they make the resuting code ugly and harder to read.  I do not think
> that the tradeoff is worth it.
>
> >       if (!strcmp("http.cookiefile", var))
> >               return git_config_pathname(&curl_cookie_file, var, value);
> >       if (!strcmp("http.savecookies", var)) {
> > @@ -924,8 +946,14 @@ static CURL *get_curl_handle(void)
> >  #if LIBCURL_VERSION_NUM >= 0x073400
> >               curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, NULL);
> >  #endif
> > -     } else if (ssl_cainfo != NULL)
> > -             curl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);
> > +     } else if (ssl_cainfo != NULL || http_proxy_ssl_ca_info != NULL) {
> > +             if (ssl_cainfo != NULL)
> > +                     curl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);
> > +#if LIBCURL_VERSION_NUM >= 0x073400
> > +             if (http_proxy_ssl_ca_info != NULL)
> > +                     curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, http_proxy_ssl_ca_info);
> > +#endif
> > +     }
>
> On this codepath, unlike the config and variable definitions,
> #if/#endif is absolutely necessary.
>
> In any case, the code around here is messy, but it is mostly due to
> the fact that the existing #if/#endif with if/elseif/... cascade was
> messy.  The general idea is
>
>  * We want to honor ssl_cainfo and http_proxy_ssl_ca_info, and use
>    CAINFO when set, but
>
>  * When http_schannel_use_ssl_cainfo is not in effect and
>    http_ssl_backend is schannel, ssl_cainfo/http_proxy_ssl_ca_info
>    business is completely skipped, and these two CAINFO are cleared
>    instead.
>
> I do not know if the above is the best code structure to express
> that, but at least the way this patch adds code is the least noisy,
> I guess.
>
> > @@ -1018,9 +1046,19 @@ static CURL *get_curl_handle(void)
> >                               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);
> > +             else if (starts_with(curl_http_proxy, "https")) {
> > +                     curl_easy_setopt(result, CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);
> > +
> > +                     if (http_proxy_ssl_cert != NULL)
> > +                             curl_easy_setopt(result, CURLOPT_PROXY_SSLCERT, http_proxy_ssl_cert);
> > +
> > +                     if (http_proxy_ssl_key != NULL)
> > +                             curl_easy_setopt(result, CURLOPT_PROXY_SSLKEY, http_proxy_ssl_key);
> > +
> > +                     if (http_proxy_ssl_keypasswd != NULL)
> > +                             curl_easy_setopt(result, CURLOPT_PROXY_KEYPASSWD, http_proxy_ssl_keypasswd);
>
> This part is more or less straight-forward.
>
> This is a minor tangent, but I see many "var != NULL" instances used
> as the condition to if statements, which we tend to frown upon
> (instead just say "if (var) ...").  I know there are already many in
> the existing code in this file, but this patch is making it even
> worse.
>
> > +             }
> >  #endif
> >               if (strstr(curl_http_proxy, "://"))
> >                       credential_from_url(&proxy_auth, curl_http_proxy);
Jorge via GitGitGadget· Mar 4, 2020, 18:40 UTC · re: Jorge via GitGitGadget · lore

[PATCH v3 0/2] Add HTTPS proxy SSL options (cert, key, cainfo)

Git currently supports connecting to proxies through HTTPS. However it does not allow you to configure SSL options when connecting (i.e. client cert, key, cainfo). These set of commits add the necessary options and documentation needed to support them.

Libcurl already has support for this so changes are somewhat minimal.

I ran the CI tests and verified manually with an HTTPS proxy that changes are working as expected. I didn't see integration tests under /t or tests that verified libcurl integration.

./bin-wrappers/git -c http.proxy=https://<PROXY-HOSTNAME> \ -c http.proxycert=<CERT> -c http.proxykey=<KEY> \ clone https://github.com/jalopezsilva/dotfiles.git

Changes since v2: =================

 * Merged the two initial commits as the second one was adding documentation
   for the first.
 * Removed the SSL Cert password from configuration. I'm using a similar
   function to has_cert_password to retrieve it if needed. 
 * Better names and descriptions were given to the options. 
 * Introduced another commit adding environment variable overrides for the
   new options.
Jorge Lopez Silva (2):
  http: add client cert for HTTPS proxies.
  http: add environment variable for HTTPS proxy.
 Documentation/config/http.txt | 21 ++++++++++
 http.c                        | 74 ++++++++++++++++++++++++++++++++---
 2 files changed, 90 insertions(+), 5 deletions(-)
base-commit: 51ebf55b9309824346a6589c9f3b130c6f371b8f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-559%2Fjalopezsilva%2Fhttps_proxy_ssl_options-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-559/jalopezsilva/https_proxy_ssl_options-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/559
Range-diff vs v2:
 1:  a5d980e7501 ! 1:  e18b342819b http: add client cert for HTTPS proxies.
     @@ -9,13 +9,44 @@
      
          A client certificate can provide an alternative way of authentication
          instead of using 'ProxyAuthorization' or other more common methods of
     -    authentication.
     +    authentication.  Libcurl supports this functionality already so changes
     +    are somewhat minimal. The feature is guarded by the first available
     +    libcurl version that supports these options.
      
     -    Libcurl supports this functionality already. The feature is guarded by
     -    the first available libcurl version that supports these options.
     +    4 configuration options are added and documented, cert, key, cert
     +    password protected and CA info. The CA info should be used to specify a
     +    different CA path to validate the HTTPS proxy cert.
      
          Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com>
      
     + diff --git a/Documentation/config/http.txt b/Documentation/config/http.txt
     + --- a/Documentation/config/http.txt
     + +++ b/Documentation/config/http.txt
     +@@
     + * `ntlm` - NTLM authentication (compare the --ntlm option of `curl(1)`)
     + --
     + 
     ++http.proxySSLCert::
     ++	The pathname of a file that stores a client certificate to use to authenticate
     ++	with an HTTPS proxy.
     ++
     ++http.proxySSLKey::
     ++	The pathname of a file that stores a private key to use to authenticate with
     ++	an HTTPS proxy.
     ++
     ++http.proxySSLCertPasswordProtected::
     ++	Enable Git's password prompt for the proxy SSL certificate.  Otherwise OpenSSL
     ++	will prompt the user, possibly many times, if the certificate or private key
     ++	is encrypted.
     ++
     ++http.proxySSLCAInfo::
     ++	Pathname to the file containing the certificate bundle that should be used to
     ++	verify the proxy with when using an HTTPS proxy.
     ++
     + http.emptyAuth::
     + 	Attempt authentication without seeking a username or password.  This
     + 	can be used to attempt GSS-Negotiate authentication without specifying
     +
       diff --git a/http.c b/http.c
       --- a/http.c
       +++ b/http.c
     @@ -24,12 +55,11 @@
       static const char *curl_http_proxy;
       static const char *http_proxy_authmethod;
      +
     -+#if LIBCURL_VERSION_NUM >= 0x073400
      +static const char *http_proxy_ssl_cert;
      +static const char *http_proxy_ssl_key;
     -+static const char *http_proxy_ssl_keypasswd;
     -+#endif
      +static const char *http_proxy_ssl_ca_info;
     ++static struct credential proxy_cert_auth = CREDENTIAL_INIT;
     ++static int proxy_ssl_cert_password_required;
      +
       static struct {
       	const char *name;
     @@ -38,23 +68,45 @@
       	if (!strcmp("http.proxyauthmethod", var))
       		return git_config_string(&http_proxy_authmethod, var, value);
       
     -+#if LIBCURL_VERSION_NUM >= 0x073400
     -+	if (!strcmp("http.proxycert", var))
     ++	if (!strcmp("http.proxysslcert", var))
      +		return git_config_string(&http_proxy_ssl_cert, var, value);
      +
     -+	if (!strcmp("http.proxykey", var))
     ++	if (!strcmp("http.proxysslkey", var))
      +		return git_config_string(&http_proxy_ssl_key, var, value);
      +
     -+	if (!strcmp("http.proxykeypass", var))
     -+		return git_config_string(&http_proxy_ssl_keypasswd, var, value);
     -+
     -+	if (!strcmp("http.proxycainfo", var))
     ++	if (!strcmp("http.proxysslcainfo", var))
      +		return git_config_string(&http_proxy_ssl_ca_info, var, value);
     -+#endif
     ++
     ++	if (!strcmp("http.proxysslcertpasswordprotected", var)) {
     ++		proxy_ssl_cert_password_required = git_config_bool(var, value);
     ++		return 0;
     ++	}
      +
       	if (!strcmp("http.cookiefile", var))
       		return git_config_pathname(&curl_cookie_file, var, value);
       	if (!strcmp("http.savecookies", var)) {
     +@@
     + 	return 1;
     + }
     + 
     ++#if LIBCURL_VERSION_NUM >= 0x073400
     ++static int has_proxy_cert_password(void)
     ++{
     ++	if (http_proxy_ssl_cert == NULL || proxy_ssl_cert_password_required != 1)
     ++		return 0;
     ++	if (!proxy_cert_auth.password) {
     ++		proxy_cert_auth.protocol = xstrdup("cert");
     ++		proxy_cert_auth.username = xstrdup("");
     ++		proxy_cert_auth.path = xstrdup(http_proxy_ssl_cert);
     ++		credential_fill(&proxy_cert_auth);
     ++	}
     ++	return 1;
     ++}
     ++#endif
     ++
     + #if LIBCURL_VERSION_NUM >= 0x071900
     + static void set_curl_keepalive(CURL *c)
     + {
      @@
       #if LIBCURL_VERSION_NUM >= 0x073400
       		curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, NULL);
     @@ -82,16 +134,28 @@
      +		else if (starts_with(curl_http_proxy, "https")) {
      +			curl_easy_setopt(result, CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);
      +
     -+			if (http_proxy_ssl_cert != NULL)
     ++			if (http_proxy_ssl_cert)
      +				curl_easy_setopt(result, CURLOPT_PROXY_SSLCERT, http_proxy_ssl_cert);
      +
     -+			if (http_proxy_ssl_key != NULL)
     ++			if (http_proxy_ssl_key)
      +				curl_easy_setopt(result, CURLOPT_PROXY_SSLKEY, http_proxy_ssl_key);
      +
     -+			if (http_proxy_ssl_keypasswd != NULL)
     -+				curl_easy_setopt(result, CURLOPT_PROXY_KEYPASSWD, http_proxy_ssl_keypasswd);
     -+
     ++			if (has_proxy_cert_password())
     ++				curl_easy_setopt(result, CURLOPT_PROXY_KEYPASSWD, proxy_cert_auth.password);
      +		}
       #endif
       		if (strstr(curl_http_proxy, "://"))
       			credential_from_url(&proxy_auth, curl_http_proxy);
     +@@
     + 	}
     + 	ssl_cert_password_required = 0;
     + 
     ++	if (proxy_cert_auth.password != NULL) {
     ++		memset(proxy_cert_auth.password, 0, strlen(proxy_cert_auth.password));
     ++		FREE_AND_NULL(proxy_cert_auth.password);
     ++	}
     ++	proxy_ssl_cert_password_required = 0;
     ++
     + 	FREE_AND_NULL(cached_accept_language);
     + }
     + 
 2:  c40207a3928 ! 2:  086c5e59fb2 config: documentation for HTTPS proxy client cert.
     @@ -1,10 +1,12 @@
      Author: Jorge Lopez Silva <jalopezsilva@gmail.com>
      
     -    config: documentation for HTTPS proxy client cert.
     +    http: add environment variable for HTTPS proxy.
      
     -    The commit adds 4 options, client cert, key, key password and CA info.
     -    The CA info can be used to specify a different CA path to validate the
     -    HTTPS proxy cert.
     +    This commit adds four environment variables that can be used to
     +    configure the proxy cert, proxy ssl key, the proxy cert password
     +    protected flag, and the CA info for the proxy.
     +
     +    Documentation for the options was also updated.
      
          Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com>
      
     @@ -12,23 +14,49 @@
       --- a/Documentation/config/http.txt
       +++ b/Documentation/config/http.txt
      @@
     - * `ntlm` - NTLM authentication (compare the --ntlm option of `curl(1)`)
     - --
       
     -+http.proxycert::
     -+	File indicating a client certificate to use to authenticate with an HTTPS proxy.
     -+
     -+http.proxykey::
     -+	File indicating a private key to use to authenticate with an HTTPS proxy.
     -+
     -+http.proxykeypass::
     -+	When communicating to the proxy using TLS (using an HTTPS proxy), use this
     -+	option along `http.proxykey` to indicate a password for the key.
     -+
     -+http.proxycainfo::
     -+	File containing the certificates to verify the proxy with when using an HTTPS
     -+	proxy.
     -+
     + http.proxySSLCert::
     + 	The pathname of a file that stores a client certificate to use to authenticate
     +-	with an HTTPS proxy.
     ++	with an HTTPS proxy. Can be overridden by the `GIT_PROXY_SSL_CERT` environment
     ++	variable.
     + 
     + http.proxySSLKey::
     + 	The pathname of a file that stores a private key to use to authenticate with
     +-	an HTTPS proxy.
     ++	an HTTPS proxy. Can be overridden by the `GIT_PROXY_SSL_KEY` environment
     ++	variable.
     + 
     + http.proxySSLCertPasswordProtected::
     + 	Enable Git's password prompt for the proxy SSL certificate.  Otherwise OpenSSL
     + 	will prompt the user, possibly many times, if the certificate or private key
     +-	is encrypted.
     ++	is encrypted. Can be overriden by the `GIT_PROXY_SSL_CERT_PASSWORD_PROTECTED`
     ++	environment variable.
     + 
     + http.proxySSLCAInfo::
     + 	Pathname to the file containing the certificate bundle that should be used to
     +-	verify the proxy with when using an HTTPS proxy.
     ++	verify the proxy with when using an HTTPS proxy. Can be overriden by the
     ++	`GIT_PROXY_SSL_CAINFO` environment variable.
     + 
       http.emptyAuth::
       	Attempt authentication without seeking a username or password.  This
     - 	can be used to attempt GSS-Negotiate authentication without specifying
     +
     + diff --git a/http.c b/http.c
     + --- a/http.c
     + +++ b/http.c
     +@@
     + 		max_requests = DEFAULT_MAX_REQUESTS;
     + #endif
     + 
     ++	set_from_env(&http_proxy_ssl_cert, "GIT_PROXY_SSL_CERT");
     ++	set_from_env(&http_proxy_ssl_key, "GIT_PROXY_SSL_KEY");
     ++	set_from_env(&http_proxy_ssl_ca_info, "GIT_PROXY_SSL_CAINFO");
     ++
     ++	if (getenv("GIT_PROXY_SSL_CERT_PASSWORD_PROTECTED"))
     ++		proxy_ssl_cert_password_required = 1;
     ++
     + 	if (getenv("GIT_CURL_FTP_NO_EPSV"))
     + 		curl_ftp_no_epsv = 1;
     + 
-- 
gitgitgadget
Jorge Lopez Silva via GitGitGadget· Mar 4, 2020, 18:40 UTC · re: Jorge via GitGitGadget · lore

[PATCH v3 1/2] http: add client cert for HTTPS proxies.

From: Jorge Lopez Silva <jalopezsilva@gmail.com>

Git currently supports performing connections to HTTPS proxies but we don't support doing mutual authentication with them (through TLS). This commit adds the necessary options to be able to send a client certificate to the HTTPS proxy.

A client certificate can provide an alternative way of authentication instead of using 'ProxyAuthorization' or other more common methods of authentication. Libcurl supports this functionality already so changes are somewhat minimal. The feature is guarded by the first available libcurl version that supports these options.

4 configuration options are added and documented, cert, key, cert password protected and CA info. The CA info should be used to specify a different CA path to validate the HTTPS proxy cert.

Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com>
---
 Documentation/config/http.txt | 17 +++++++++
 http.c                        | 67 ++++++++++++++++++++++++++++++++---
 2 files changed, 79 insertions(+), 5 deletions(-)
Show changes to 2 files +79 −5

Documentation/config/http.txt, http.c

diff --git a/Documentation/config/http.txt b/Documentation/config/http.txt
index e806033aab8..7d398f9afba 100644
--- a/Documentation/config/http.txt
+++ b/Documentation/config/http.txt
@@ -29,6 +29,23 @@ http.proxyAuthMethod::
 * `ntlm` - NTLM authentication (compare the --ntlm option of `curl(1)`)
 --
 
+http.proxySSLCert::
+	The pathname of a file that stores a client certificate to use to authenticate
+	with an HTTPS proxy.
+
+http.proxySSLKey::
+	The pathname of a file that stores a private key to use to authenticate with
+	an HTTPS proxy.
+
+http.proxySSLCertPasswordProtected::
+	Enable Git's password prompt for the proxy SSL certificate.  Otherwise OpenSSL
+	will prompt the user, possibly many times, if the certificate or private key
+	is encrypted.
+
+http.proxySSLCAInfo::
+	Pathname to the file containing the certificate bundle that should be used to
+	verify the proxy with when using an HTTPS proxy.
+
 http.emptyAuth::
 	Attempt authentication without seeking a username or password.  This
 	can be used to attempt GSS-Negotiate authentication without specifying
diff --git a/http.c b/http.c
index 00a0e507633..8d616b5d60e 100644
--- a/http.c
+++ b/http.c
@@ -86,6 +86,13 @@ static long curl_low_speed_time = -1;
 static int curl_ftp_no_epsv;
 static const char *curl_http_proxy;
 static const char *http_proxy_authmethod;
+
+static const char *http_proxy_ssl_cert;
+static const char *http_proxy_ssl_key;
+static const char *http_proxy_ssl_ca_info;
+static struct credential proxy_cert_auth = CREDENTIAL_INIT;
+static int proxy_ssl_cert_password_required;
+
 static struct {
 	const char *name;
 	long curlauth_param;
@@ -365,6 +372,20 @@ static int http_options(const char *var, const char *value, void *cb)
 	if (!strcmp("http.proxyauthmethod", var))
 		return git_config_string(&http_proxy_authmethod, var, value);
 
+	if (!strcmp("http.proxysslcert", var))
+		return git_config_string(&http_proxy_ssl_cert, var, value);
+
+	if (!strcmp("http.proxysslkey", var))
+		return git_config_string(&http_proxy_ssl_key, var, value);
+
+	if (!strcmp("http.proxysslcainfo", var))
+		return git_config_string(&http_proxy_ssl_ca_info, var, value);
+
+	if (!strcmp("http.proxysslcertpasswordprotected", var)) {
+		proxy_ssl_cert_password_required = git_config_bool(var, value);
+		return 0;
+	}
+
 	if (!strcmp("http.cookiefile", var))
 		return git_config_pathname(&curl_cookie_file, var, value);
 	if (!strcmp("http.savecookies", var)) {
@@ -565,6 +586,21 @@ static int has_cert_password(void)
 	return 1;
 }
 
+#if LIBCURL_VERSION_NUM >= 0x073400
+static int has_proxy_cert_password(void)
+{
+	if (http_proxy_ssl_cert == NULL || proxy_ssl_cert_password_required != 1)
+		return 0;
+	if (!proxy_cert_auth.password) {
+		proxy_cert_auth.protocol = xstrdup("cert");
+		proxy_cert_auth.username = xstrdup("");
+		proxy_cert_auth.path = xstrdup(http_proxy_ssl_cert);
+		credential_fill(&proxy_cert_auth);
+	}
+	return 1;
+}
+#endif
+
 #if LIBCURL_VERSION_NUM >= 0x071900
 static void set_curl_keepalive(CURL *c)
 {
@@ -924,8 +960,14 @@ static CURL *get_curl_handle(void)
 #if LIBCURL_VERSION_NUM >= 0x073400
 		curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, NULL);
 #endif
-	} else if (ssl_cainfo != NULL)
-		curl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);
+	} else if (ssl_cainfo != NULL || http_proxy_ssl_ca_info != NULL) {
+		if (ssl_cainfo != NULL)
+			curl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);
+#if LIBCURL_VERSION_NUM >= 0x073400
+		if (http_proxy_ssl_ca_info != NULL)
+			curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, http_proxy_ssl_ca_info);
+#endif
+	}
 
 	if (curl_low_speed_limit > 0 && curl_low_speed_time > 0) {
 		curl_easy_setopt(result, CURLOPT_LOW_SPEED_LIMIT,
@@ -1018,9 +1060,18 @@ static CURL *get_curl_handle(void)
 				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);
+		else if (starts_with(curl_http_proxy, "https")) {
+			curl_easy_setopt(result, CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);
+
+			if (http_proxy_ssl_cert)
+				curl_easy_setopt(result, CURLOPT_PROXY_SSLCERT, http_proxy_ssl_cert);
+
+			if (http_proxy_ssl_key)
+				curl_easy_setopt(result, CURLOPT_PROXY_SSLKEY, http_proxy_ssl_key);
+
+			if (has_proxy_cert_password())
+				curl_easy_setopt(result, CURLOPT_PROXY_KEYPASSWD, proxy_cert_auth.password);
+		}
 #endif
 		if (strstr(curl_http_proxy, "://"))
 			credential_from_url(&proxy_auth, curl_http_proxy);
@@ -1230,6 +1281,12 @@ void http_cleanup(void)
 	}
 	ssl_cert_password_required = 0;
 
+	if (proxy_cert_auth.password != NULL) {
+		memset(proxy_cert_auth.password, 0, strlen(proxy_cert_auth.password));
+		FREE_AND_NULL(proxy_cert_auth.password);
+	}
+	proxy_ssl_cert_password_required = 0;
+
 	FREE_AND_NULL(cached_accept_language);
 }
 
-- 
gitgitgadget
Jorge Lopez Silva via GitGitGadget· Mar 4, 2020, 18:40 UTC · re: Jorge via GitGitGadget · lore

[PATCH v3 2/2] http: add environment variable for HTTPS proxy.

From: Jorge Lopez Silva <jalopezsilva@gmail.com>

This commit adds four environment variables that can be used to configure the proxy cert, proxy ssl key, the proxy cert password protected flag, and the CA info for the proxy.

Documentation for the options was also updated.
Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com>
---
 Documentation/config/http.txt | 12 ++++++++----
 http.c                        |  7 +++++++
 2 files changed, 15 insertions(+), 4 deletions(-)
Show changes to 2 files +15 −4

Documentation/config/http.txt, http.c

diff --git a/Documentation/config/http.txt b/Documentation/config/http.txt
index 7d398f9afba..3968fbb697a 100644
--- a/Documentation/config/http.txt
+++ b/Documentation/config/http.txt
@@ -31,20 +31,24 @@ http.proxyAuthMethod::
 
 http.proxySSLCert::
 	The pathname of a file that stores a client certificate to use to authenticate
-	with an HTTPS proxy.
+	with an HTTPS proxy. Can be overridden by the `GIT_PROXY_SSL_CERT` environment
+	variable.
 
 http.proxySSLKey::
 	The pathname of a file that stores a private key to use to authenticate with
-	an HTTPS proxy.
+	an HTTPS proxy. Can be overridden by the `GIT_PROXY_SSL_KEY` environment
+	variable.
 
 http.proxySSLCertPasswordProtected::
 	Enable Git's password prompt for the proxy SSL certificate.  Otherwise OpenSSL
 	will prompt the user, possibly many times, if the certificate or private key
-	is encrypted.
+	is encrypted. Can be overriden by the `GIT_PROXY_SSL_CERT_PASSWORD_PROTECTED`
+	environment variable.
 
 http.proxySSLCAInfo::
 	Pathname to the file containing the certificate bundle that should be used to
-	verify the proxy with when using an HTTPS proxy.
+	verify the proxy with when using an HTTPS proxy. Can be overriden by the
+	`GIT_PROXY_SSL_CAINFO` environment variable.
 
 http.emptyAuth::
 	Attempt authentication without seeking a username or password.  This
diff --git a/http.c b/http.c
index 8d616b5d60e..4283be9479b 100644
--- a/http.c
+++ b/http.c
@@ -1211,6 +1211,13 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)
 		max_requests = DEFAULT_MAX_REQUESTS;
 #endif
 
+	set_from_env(&http_proxy_ssl_cert, "GIT_PROXY_SSL_CERT");
+	set_from_env(&http_proxy_ssl_key, "GIT_PROXY_SSL_KEY");
+	set_from_env(&http_proxy_ssl_ca_info, "GIT_PROXY_SSL_CAINFO");
+
+	if (getenv("GIT_PROXY_SSL_CERT_PASSWORD_PROTECTED"))
+		proxy_ssl_cert_password_required = 1;
+
 	if (getenv("GIT_CURL_FTP_NO_EPSV"))
 		curl_ftp_no_epsv = 1;
 
-- 
gitgitgadget

← back to recent threads