git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Nov 4, 2015, 19:41 UTC
Message-ID
<CAPig+cS9PtibBQYAPXmG1GEBAMfd741FApYo8YnsNeVzw-Wt-A@mail.gmail.com>
In-Reply-To
<1446628405-8070-3-git-send-email-k.franke@science-computing.de>

On Wednesday, November 4, 2015, Knut Franke <k.franke@science-computing.de> wrote:

Show 34 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>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> @@ -1593,9 +1593,13 @@ help.htmlPath::
>
>  http.proxy::
>         Override the HTTP proxy, normally configured using the 'http_proxy',
> -       'https_proxy', and 'all_proxy' environment variables (see
> -       `curl(1)`).  This can be overridden on a per-remote basis; see
> -       remote.<name>.proxy
> +       'https_proxy', and 'all_proxy' environment variables (see `curl(1)`). In
> +       addition to the syntax understood by curl, it is possible to specify a
> +       proxy string with a user name but no password, in which case git will
> +       attempt to acquire one in the same way it does for other credentials. See
> +       linkgit:gitcredentials[7] for more information. The syntax thus is
> +       '[protocol://][user[:password]@]proxyhost[:port]'. This can be overridden
> +       on a per-remote basis; see remote.<name>.proxy
s/$/./ maybe?
Show 16 quoted lines
>  http.proxyAuthMethod::
>         Set the method with which to authenticate against the HTTP proxy. This
> diff --git a/http.c b/http.c
> @@ -337,6 +342,24 @@ static void var_override(const char **var, char *value)
>
>  static void init_curl_proxy_auth(CURL *result)
>  {
> +       if (proxy_auth.username) {
> +               struct strbuf s = STRBUF_INIT;
> +               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);

The strbuf does not get released in this #if branch, but since no content was added, it doesn't need to be released, thus nothing is leaked. Good.

Does the compiler warn about unused variable 's' in this #if branch?
Show 6 quoted lines
> +#else
> +               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);

And this branch detaches content from the strbuf, and that memory is released later in http_cleanup(), so also nothing is leaked, thus all is good.

Show 6 quoted lines
> +#endif
> +       }
> +
>         var_override(&http_proxy_authmethod, getenv("GIT_HTTP_PROXY_AUTHMETHOD"));
>
>  #if LIBCURL_VERSION_NUM >= 0x070a07 /* CURLOPT_PROXYAUTH and CURLAUTH_ANY */
Previous: Knut FrankeNext: Junio C Hamano
Message 30 of 34 in “http: allow selection of proxy authentication method”
  1. 1/2 http: allow selection of proxy authentication methodKnut Franke, Oct 26, 2015
  2. 2/2 http: use credential API to handle proxy authenticationKnut Franke, Oct 26, 2015
  3. Junio C HamanoOct 26, 2015
  4. Knut FrankeOct 27, 2015
  5. http proxy authentication improvementsKnut Franke, Oct 28, 2015
  6. 1/2 http: allow selection of proxy authentication methodKnut Franke, Oct 28, 2015
  7. Junio C HamanoOct 28, 2015
  8. Junio C HamanoOct 28, 2015
  9. Knut FrankeOct 30, 2015
  10. Junio C HamanoOct 30, 2015
  11. Eric SunshineOct 28, 2015
  12. 2/2 http: use credential API to handle proxy authenticationKnut Franke, Oct 28, 2015
  13. Eric SunshineOct 28, 2015
  14. Knut FrankeOct 30, 2015
  15. Junio C HamanoOct 30, 2015
  16. Eric SunshineOct 30, 2015
  17. 0/2 Knut Franke, Nov 2, 2015
  18. 1/2 http: allow selection of proxy authentication methodKnut Franke, Nov 2, 2015
  19. Junio C HamanoNov 2, 2015
  20. Knut FrankeNov 3, 2015
  21. Junio C HamanoNov 3, 2015
  22. 2/2 http: use credential API to handle proxy authenticationKnut Franke, Nov 2, 2015
  23. Junio C HamanoNov 2, 2015
  24. Knut FrankeNov 3, 2015
  25. Eric SunshineNov 3, 2015
  26. 0/2 Knut Franke, Nov 4, 2015
  27. 1/2 http: allow selection of proxy authentication methodKnut Franke, Nov 4, 2015
  28. Junio C HamanoNov 4, 2015
  29. 2/2 http: use credential API to handle proxy authenticationKnut Franke, Nov 4, 2015
  30. Eric SunshineNov 4, 2015
  31. Junio C HamanoNov 4, 2015
  32. Jeff KingNov 5, 2015
  33. Knut FrankeNov 5, 2015
  34. Jeff KingNov 5, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.