Re: [PATCH 2/2] http: use credential API to handle proxy authentication
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 2, 2015, 22:54 UTC
- Message-ID
- <xmqqbnbcdnb8.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <1446483264-15123-3-git-send-email-k.franke@science-computing.de>
Knut Franke <k.franke@science-computing.de> writes:
Show 18 quoted lines
> Currently, the only way to pass proxy credentials to curl is by including them > in the proxy URL. Usually, this means they will end up on disk unencrypted, one > way or another (by inclusion in ~/.gitconfig, shell profile or history). Since > proxy authentication often uses a domain user, credentials can be security > sensitive; therefore, a safer way of passing credentials is desirable. > > If the configured proxy contains a username but not a password, query the > credential API for one. Also, make sure we approve/reject proxy credentials > properly. > > For consistency reasons, add parsing of http_proxy/https_proxy/all_proxy > environment variables, which would otherwise be evaluated as a fallback by curl. > Without this, we would have different semantics for git configuration and > environment variables. > > Signed-off-by: Knut Franke <k.franke@science-computing.de> > Reviewed-by: Junio C Hamano <gitster@pobox.com> > Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>
As 1/2, I never reviewed this version yet.
Show 16 quoted lines
> --- > http.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- > http.h | 1 + > 2 files changed, 75 insertions(+), 2 deletions(-) > > diff --git a/http.c b/http.c > index 1172819..5708c7a 100644 > --- a/http.c > +++ b/http.c > @@ -62,7 +62,7 @@ static const char *ssl_cainfo; > static long curl_low_speed_limit = -1; > static long curl_low_speed_time = -1; > static int curl_ftp_no_epsv; > -static const char *curl_http_proxy; > +static const char *curl_http_proxy = NULL; > static const char *http_proxy_authmethod = NULL;
We do not do unnecessary initialization of file-scope globals to 0 or NULL. The existing definition of curl_http_proxy is correct; the one for http_proxy_authmethod needs to be changed to match.
Show 18 quoted lines
> static void init_curl_proxy_auth(CURL *result)
> {
> + if (proxy_auth.username) {
> + if (!proxy_auth.password)
> + credential_fill(&proxy_auth);
> +#if LIBCURL_VERSION_NUM >= 0x071301
> + curl_easy_setopt(result, CURLOPT_PROXYUSERNAME,
> + proxy_auth.username);
> + curl_easy_setopt(result, CURLOPT_PROXYPASSWORD,
> + proxy_auth.password);
> +#else
> + struct strbuf s = STRBUF_INIT;
> + strbuf_addstr_urlencode(&s, proxy_auth.username, 1);
> + strbuf_addch(&s, ':');
> + strbuf_addstr_urlencode(&s, proxy_auth.password, 1);
> + curl_proxyuserpwd = strbuf_detach(&s, NULL);
> + curl_easy_setopt(result, CURLOPT_PROXYUSERPWD, curl_proxyuserpwd);
> +#endifI think #else clause of this thing would introduce decl-after-stmt compilation error.