* [PATCH 0/2] Add HTTPS proxy SSL options (cert, key, cainfo)
@ 2020-02-21 21:36 Jorge via GitGitGadget
2020-02-21 21:36 ` [PATCH 1/2] http: add client cert for HTTPS proxies Jorge Lopez Silva via GitGitGadget
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Jorge via GitGitGadget @ 2020-02-21 21:36 UTC (permalink / raw)
To: git; +Cc: Jorge
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
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/2] http: add client cert for HTTPS proxies. 2020-02-21 21:36 [PATCH 0/2] Add HTTPS proxy SSL options (cert, key, cainfo) Jorge via GitGitGadget @ 2020-02-21 21:36 ` Jorge Lopez Silva via GitGitGadget 2020-02-21 22:28 ` Eric Sunshine 2020-02-21 21:36 ` [PATCH 2/2] config: documentation for HTTPS proxy client cert Jorge Lopez Silva via GitGitGadget 2020-02-26 23:23 ` [PATCH v2 0/2] Add HTTPS proxy SSL options (cert, key, cainfo) Jorge via GitGitGadget 2 siblings, 1 reply; 15+ messages in thread From: Jorge Lopez Silva via GitGitGadget @ 2020-02-21 21:36 UTC (permalink / raw) To: git; +Cc: Jorge, Jorge Lopez Silva 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(-) 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] http: add client cert for HTTPS proxies. 2020-02-21 21:36 ` [PATCH 1/2] http: add client cert for HTTPS proxies Jorge Lopez Silva via GitGitGadget @ 2020-02-21 22:28 ` Eric Sunshine 2020-02-26 21:05 ` Jorge A López Silva 0 siblings, 1 reply; 15+ messages in thread From: Eric Sunshine @ 2020-02-21 22:28 UTC (permalink / raw) To: Jorge Lopez Silva via GitGitGadget; +Cc: Git List, Jorge 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] http: add client cert for HTTPS proxies. 2020-02-21 22:28 ` Eric Sunshine @ 2020-02-26 21:05 ` Jorge A López Silva 0 siblings, 0 replies; 15+ messages in thread From: Jorge A López Silva @ 2020-02-26 21:05 UTC (permalink / raw) To: Eric Sunshine; +Cc: Jorge Lopez Silva via GitGitGadget, Git List 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: > > 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/2] config: documentation for HTTPS proxy client cert. 2020-02-21 21:36 [PATCH 0/2] Add HTTPS proxy SSL options (cert, key, cainfo) Jorge via GitGitGadget 2020-02-21 21:36 ` [PATCH 1/2] http: add client cert for HTTPS proxies Jorge Lopez Silva via GitGitGadget @ 2020-02-21 21:36 ` Jorge Lopez Silva via GitGitGadget 2020-02-26 23:23 ` [PATCH v2 0/2] Add HTTPS proxy SSL options (cert, key, cainfo) Jorge via GitGitGadget 2 siblings, 0 replies; 15+ messages in thread From: Jorge Lopez Silva via GitGitGadget @ 2020-02-21 21:36 UTC (permalink / raw) To: git; +Cc: Jorge, Jorge Lopez Silva 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(+) 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 0/2] Add HTTPS proxy SSL options (cert, key, cainfo) 2020-02-21 21:36 [PATCH 0/2] Add HTTPS proxy SSL options (cert, key, cainfo) Jorge via GitGitGadget 2020-02-21 21:36 ` [PATCH 1/2] http: add client cert for HTTPS proxies Jorge Lopez Silva via GitGitGadget 2020-02-21 21:36 ` [PATCH 2/2] config: documentation for HTTPS proxy client cert Jorge Lopez Silva via GitGitGadget @ 2020-02-26 23:23 ` Jorge via GitGitGadget 2020-02-26 23:23 ` [PATCH v2 1/2] http: add client cert for HTTPS proxies Jorge Lopez Silva via GitGitGadget ` (2 more replies) 2 siblings, 3 replies; 15+ messages in thread From: Jorge via GitGitGadget @ 2020-02-26 23:23 UTC (permalink / raw) To: git; +Cc: Jorge 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/2] http: add client cert for HTTPS proxies. 2020-02-26 23:23 ` [PATCH v2 0/2] Add HTTPS proxy SSL options (cert, key, cainfo) Jorge via GitGitGadget @ 2020-02-26 23:23 ` Jorge Lopez Silva via GitGitGadget 2020-02-27 18:31 ` Junio C Hamano 2020-02-26 23:23 ` [PATCH v2 2/2] config: documentation for HTTPS proxy client cert Jorge Lopez Silva via GitGitGadget 2020-03-04 18:40 ` [PATCH v3 0/2] Add HTTPS proxy SSL options (cert, key, cainfo) Jorge via GitGitGadget 2 siblings, 1 reply; 15+ messages in thread From: Jorge Lopez Silva via GitGitGadget @ 2020-02-26 23:23 UTC (permalink / raw) To: git; +Cc: Jorge, Jorge Lopez Silva 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(-) 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] http: add client cert for HTTPS proxies. 2020-02-26 23:23 ` [PATCH v2 1/2] http: add client cert for HTTPS proxies Jorge Lopez Silva via GitGitGadget @ 2020-02-27 18:31 ` Junio C Hamano 2020-03-03 1:41 ` Jorge A López Silva 0 siblings, 1 reply; 15+ messages in thread From: Junio C Hamano @ 2020-02-27 18:31 UTC (permalink / raw) To: Jorge Lopez Silva via GitGitGadget; +Cc: git, Jorge "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); ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] http: add client cert for HTTPS proxies. 2020-02-27 18:31 ` Junio C Hamano @ 2020-03-03 1:41 ` Jorge A López Silva 0 siblings, 0 replies; 15+ messages in thread From: Jorge A López Silva @ 2020-03-03 1:41 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jorge Lopez Silva via GitGitGadget, Git List > 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. > 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: > > "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); ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/2] config: documentation for HTTPS proxy client cert. 2020-02-26 23:23 ` [PATCH v2 0/2] Add HTTPS proxy SSL options (cert, key, cainfo) Jorge via GitGitGadget 2020-02-26 23:23 ` [PATCH v2 1/2] http: add client cert for HTTPS proxies Jorge Lopez Silva via GitGitGadget @ 2020-02-26 23:23 ` Jorge Lopez Silva via GitGitGadget 2020-02-27 18:58 ` Junio C Hamano 2020-03-04 18:40 ` [PATCH v3 0/2] Add HTTPS proxy SSL options (cert, key, cainfo) Jorge via GitGitGadget 2 siblings, 1 reply; 15+ messages in thread From: Jorge Lopez Silva via GitGitGadget @ 2020-02-26 23:23 UTC (permalink / raw) To: git; +Cc: Jorge, Jorge Lopez Silva 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(+) 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] config: documentation for HTTPS proxy client cert. 2020-02-26 23:23 ` [PATCH v2 2/2] config: documentation for HTTPS proxy client cert Jorge Lopez Silva via GitGitGadget @ 2020-02-27 18:58 ` Junio C Hamano 2020-03-03 1:47 ` Jorge A López Silva 0 siblings, 1 reply; 15+ messages in thread From: Junio C Hamano @ 2020-02-27 18:58 UTC (permalink / raw) To: Jorge Lopez Silva via GitGitGadget; +Cc: git, Jorge "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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] config: documentation for HTTPS proxy client cert. 2020-02-27 18:58 ` Junio C Hamano @ 2020-03-03 1:47 ` Jorge A López Silva 0 siblings, 0 replies; 15+ messages in thread From: Jorge A López Silva @ 2020-03-03 1:47 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jorge Lopez Silva via GitGitGadget, Git List > 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. > 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. > 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: > > "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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 0/2] Add HTTPS proxy SSL options (cert, key, cainfo) 2020-02-26 23:23 ` [PATCH v2 0/2] Add HTTPS proxy SSL options (cert, key, cainfo) Jorge via GitGitGadget 2020-02-26 23:23 ` [PATCH v2 1/2] http: add client cert for HTTPS proxies Jorge Lopez Silva via GitGitGadget 2020-02-26 23:23 ` [PATCH v2 2/2] config: documentation for HTTPS proxy client cert Jorge Lopez Silva via GitGitGadget @ 2020-03-04 18:40 ` Jorge via GitGitGadget 2020-03-04 18:40 ` [PATCH v3 1/2] http: add client cert for HTTPS proxies Jorge Lopez Silva via GitGitGadget 2020-03-04 18:40 ` [PATCH v3 2/2] http: add environment variable for HTTPS proxy Jorge Lopez Silva via GitGitGadget 2 siblings, 2 replies; 15+ messages in thread From: Jorge via GitGitGadget @ 2020-03-04 18:40 UTC (permalink / raw) To: git; +Cc: Jorge 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 1/2] http: add client cert for HTTPS proxies. 2020-03-04 18:40 ` [PATCH v3 0/2] Add HTTPS proxy SSL options (cert, key, cainfo) Jorge via GitGitGadget @ 2020-03-04 18:40 ` Jorge Lopez Silva via GitGitGadget 2020-03-04 18:40 ` [PATCH v3 2/2] http: add environment variable for HTTPS proxy Jorge Lopez Silva via GitGitGadget 1 sibling, 0 replies; 15+ messages in thread From: Jorge Lopez Silva via GitGitGadget @ 2020-03-04 18:40 UTC (permalink / raw) To: git; +Cc: Jorge, Jorge Lopez Silva 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(-) 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 2/2] http: add environment variable for HTTPS proxy. 2020-03-04 18:40 ` [PATCH v3 0/2] Add HTTPS proxy SSL options (cert, key, cainfo) Jorge via GitGitGadget 2020-03-04 18:40 ` [PATCH v3 1/2] http: add client cert for HTTPS proxies Jorge Lopez Silva via GitGitGadget @ 2020-03-04 18:40 ` Jorge Lopez Silva via GitGitGadget 1 sibling, 0 replies; 15+ messages in thread From: Jorge Lopez Silva via GitGitGadget @ 2020-03-04 18:40 UTC (permalink / raw) To: git; +Cc: Jorge, Jorge Lopez Silva 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(-) 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-03-04 18:40 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-21 21:36 [PATCH 0/2] Add HTTPS proxy SSL options (cert, key, cainfo) Jorge via GitGitGadget 2020-02-21 21:36 ` [PATCH 1/2] http: add client cert for HTTPS proxies Jorge Lopez Silva via GitGitGadget 2020-02-21 22:28 ` Eric Sunshine 2020-02-26 21:05 ` Jorge A López Silva 2020-02-21 21:36 ` [PATCH 2/2] config: documentation for HTTPS proxy client cert Jorge Lopez Silva via GitGitGadget 2020-02-26 23:23 ` [PATCH v2 0/2] Add HTTPS proxy SSL options (cert, key, cainfo) Jorge via GitGitGadget 2020-02-26 23:23 ` [PATCH v2 1/2] http: add client cert for HTTPS proxies Jorge Lopez Silva via GitGitGadget 2020-02-27 18:31 ` Junio C Hamano 2020-03-03 1:41 ` Jorge A López Silva 2020-02-26 23:23 ` [PATCH v2 2/2] config: documentation for HTTPS proxy client cert Jorge Lopez Silva via GitGitGadget 2020-02-27 18:58 ` Junio C Hamano 2020-03-03 1:47 ` Jorge A López Silva 2020-03-04 18:40 ` [PATCH v3 0/2] Add HTTPS proxy SSL options (cert, key, cainfo) Jorge via GitGitGadget 2020-03-04 18:40 ` [PATCH v3 1/2] http: add client cert for HTTPS proxies Jorge Lopez Silva via GitGitGadget 2020-03-04 18:40 ` [PATCH v3 2/2] http: add environment variable for HTTPS proxy Jorge Lopez Silva via GitGitGadget
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).