Thread (18 messages) flat view 18 messages, 3 authors, 2016-06-15

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

From: Eric Sunshine <hidden>
Date: 2016-06-15 23:07:11

On Wednesday, November 4, 2015, Knut Franke
[off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
Helped-by: Junio C Hamano [off-list ref]
Helped-by: Eric Sunshine [off-list ref]
---
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?
quoted hunk ↗ jump to hunk
 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?
+#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.
+#endif
+       }
+
        var_override(&http_proxy_authmethod, getenv("GIT_HTTP_PROXY_AUTHMETHOD"));

 #if LIBCURL_VERSION_NUM >= 0x070a07 /* CURLOPT_PROXYAUTH and CURLAUTH_ANY */
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help