Re: [PATCH 2/2] http: use credential API to handle proxy authentication

[Junio C Hamano <gitster@pobox.com>]

Knut Franke <k.franke@science-computing.de> writes:

> 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.

> ---
>  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.

>  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);
> +#endif

I think #else clause of this thing would introduce decl-after-stmt
compilation error.