Re: [PATCH 2/2] http: use credential API to handle proxy authentication
On 2015-11-05 03:24, Jeff King wrote:
> There was also some discussion with curl upstream of providing a new
> authentication interface, where we would provide curl with
> authentication callbacks, and it would trigger them if and when
> credentials were needed. Somebody upstream was working on a patch, but I
> don't think it ever got merged. :(
That would certainly be nice, also with respect to other credentials, such as
SSL key passphrase (presuming that'd be possible without modifying the SSL lib
as well).
> Here's a relevant bit from that old series (which doesn't seem threaded,
> but you can search for the author if you want to see more):
>
> http://thread.gmane.org/gmane.comp.version-control.git/192246
My main takeaway from this, apart from the points you mention below, is that
it'd be good to have a test case, similar to t/lib-httpd.sh. Since none of the
existent proxy-related code has an automated test, I think this would be an
improvement on top of the other patches. I'd need to look into how easy/hard
this would be to implement.
> > +
> > + curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CONNECTCODE,
> > + &slot->results->http_connectcode);
>
> It looks like you use this to see the remote side's HTTP 407 code. In
> the 2012 series, I think we simply looked for a 407 in the HTTP return
> code
I'm not sure why that worked for the author of the old series - possibly curl
semantics changed at some point. In my setup at least (with curl 7.15.5), after
a failed proxy authentication, CURLINFO_HTTP_CODE returns 0 while
CURLINFO_HTTP_CONNECTCODE returns the 407. This is also consistent with the curl
documentation for CURLINFO_RESPONSE_CODE (which has replaced CURLINFO_HTTP_CODE
in 7.10.7, though the compatibility #define is still there): "Note that a
proxy's CONNECT response should be read with CURLINFO_HTTP_CONNECTCODE and not
this."
> If we do need CONNECTCODE, do we need to protect it with an #ifdef on
> the curl version? The manpage says it came in 7.10.7, which was released
> in 2003. That's probably old enough not to worry about.
As Junio pointed out earlier, since some people still care about ancient curl
versions, we don't want to knowingly break compatibility. So yes, an #ifdef
would be in oder here.
>
> > + if (proxy_auth.password) {
> > + memset(proxy_auth.password, 0, strlen(proxy_auth.password));
> > + free(proxy_auth.password);
>
> My understanding is that memset() like this is not sufficient for
> zero-ing sensitive data, as they can be optimized out by the compiler. I
> don't think there's a portable alternative, though, so it may be the
> best we can do. OTOH, the rest of git does not worry about such zero-ing
> anyway, so we could also simply omit it here.
For what it's worth, that's the same as we do for cert_auth (while, as far as I
can see, no attempt is made for http_auth). I tend to think it's better than
nothing. Maybe an in-code comment stating it's not reliable would be in order,
to prevent the passing reader from putting too much trust in it.
> > + free((void *)curl_proxyuserpwd);
>
> This cast is necessary because curl_proxyuserpwd is declared const. But
> I do not see anywhere that it needs to be const (we detach a strbuf into
> it). Can we simply change the declaration?
Right.
> For that matter, it is not clear to me why this needs to be a global at
> all. Once we hand the value to curl_easy_setopt, curl keeps its own
> copy.
That's true only for relatively recent curl versions; before 7.17.0, strings
were not copied.
> > @@ -1008,6 +1076,8 @@ static int handle_curl_result(struct slot_results
> > *results)
> > return HTTP_REAUTH;
> > }
> > } else {
> > + if (results->http_connectcode == 407)
> > + credential_reject(&proxy_auth);
>
> Rejecting on a 407 makes sense (though again, can we check
> results->http_code?). But if we get a 407 and we _don't_ have a
> password, shouldn't we then prompt for one, similar to what we do with a
> 401?
>
> That will require some refactoring around http_request_reauth, though
> (because now we might potentially retry twice: once to get past the
> proxy auth, and once to get past the real site's auth).
I think this would also require changes to post_rpc in remote-curl.c, which
apparently does something similar to http_request_reauth. Probably something
along the lines of adding a HTTP_PROXY_REAUTH return code, plus some refactoring
in order to prevent code duplication between the different code parts handling
(proxy) reauth. :-/
> You prompt unconditionally for the password earlier, but only if the
> proxy URL contains a username. We used to do the same thing for regular
> http, but people got annoyed that they had to specify half the
> credential in the URL. Perhaps it would be less so with proxies (which
> are changed a lot less), so I don't think making this work is an
> absolute requirement.
As far as I
[PATCH 2/2] http: use credential API to handle proxy authentication
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
Helped-by: Junio C Hamano
Helped-by: Eric Sunshine
---
Documentation/config.txt | 10 +--
http.c | 72 +++-
http.h | 1 +
3 files changed, 79 insertions(+), 4 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 4ad7f74..320ce9a 100644
--- 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..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..proxy
http.proxyAuthMethod::
Set the method with which to authenticate against the HTTP proxy. This
diff --git a/http.c b/http.c
index a786802..714a59a 100644
--- a/http.c
+++ b/http.c
@@ -81,6 +81,8 @@ static struct {
* here, too
*/
};
+static struct credential proxy_auth = CREDENTIAL_INIT;
+static const char *curl_proxyuserpwd;
static const char *curl_cookie_file;
static int curl_save_cookies;
struct credential http_auth = CREDENTIAL_INIT;
@@ -178,6 +180,9 @@ static void finish_active_slot(struct active_request_slot
*slot)
#else
slot->results->auth_avail = 0;
#endif
+
+ curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CONNECTCODE,
+ &slot->results->http_connectcode);
}
/* Run callback if appropriate */
@@ -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);
+#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);
+#endif
+ }
+
var_override(&http_proxy_authmethod,
getenv("GIT_HTTP_PROXY_AUTHMETHOD"));
#if LIBCURL_VERSION_NUM >= 0x070a07 /* CURLOPT_PROXYAUTH and CURLAUTH_ANY */
@@ -518,8 +541,42 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_USE_SSL, CURLUSESSL_TRY);
#endif
+ /*
+* CURL also examines these variables as a fallback; but we need to
query
+* them here in order to decide whether to prompt for missing password
(cf.
+* init_curl_proxy_auth()).
+*
+* Unlike many other common environment variables, these are
historically
+* lowercase only. It appears that CURL did not know this and
implemented
+* only uppercase variants, which was later corrected to take both -
with
+* the exception of http_proxy, which is lowercase only also in CURL. As
+* the lowercase versions are the historical quasi-standard, they take
+* precedence here, as in CURL.
+*/
+ if (!curl_http_proxy) {
+ if (!strcmp(http_auth.protocol
[PATCH v4 0/2]
Changes in the fourth iteration: * update Documentation/technical/api-remote.txt to reflect the addition to struct remote * update documentation of http.proxy in Documentation/config.txt to mention the possibility of having git fill in missing proxy password * fix decl-after-stmt * generalize env_override helper to var_override * more coding style corrections -- Vorstandsvorsitzender/Chairman of the board of management: Gerd-Lothar Leonhart Vorstand/Board of Management: Dr. Bernd Finkbeiner, Dr. Arno Steitz Vorsitzender des Aufsichtsrats/ Chairman of the Supervisory Board: Philippe Miltin Sitz/Registered Office: Tuebingen Registergericht/Registration Court: Stuttgart Registernummer/Commercial Register No.: HRB 382196 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] http: allow selection of proxy authentication method
CURLAUTH_ANY does not work with proxies which answer unauthenticated requests
with a 307 redirect to an error page instead of a 407 listing supported
authentication methods. Therefore, allow the authentication method to be set
using the environment variable GIT_HTTP_PROXY_AUTHMETHOD or configuration
variables http.proxyAuthmethod and remote..proxyAuthmethod (in analogy
to http.proxy and remote..proxy).
The following values are supported:
* anyauth (default)
* basic
* digest
* negotiate
* ntlm
Signed-off-by: Knut Franke
Helped-by: Junio C Hamano
Helped-by: Eric Sunshine
---
Documentation/config.txt | 26 ++
Documentation/technical/api-remote.txt | 4 +++
http.c | 65 --
remote.c | 3 ++
remote.h | 1 +
5 files changed, 96 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 391a0c3..4ad7f74 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1597,6 +1597,27 @@ http.proxy::
`curl(1)`). This can be overridden on a per-remote basis; see
remote..proxy
+http.proxyAuthMethod::
+ Set the method with which to authenticate against the HTTP proxy. This
+ only takes effect if the configured proxy string contains a user name
part
+ (i.e. is of the form 'user@host' or 'user@host:port'). This can be
+ overridden on a per-remote basis; see `remote..proxyAuthMethod`.
+ Both can be overridden by the 'GIT_HTTP_PROXY_AUTHMETHOD' environment
+ variable. Possible values are:
++
+--
+* `anyauth` - Automatically pick a suitable authentication method. It is
+ assumed that the proxy answers an unauthenticated request with a 407
+ status code and one or more Proxy-authenticate headers with supported
+ authentication methods. This is the default.
+* `basic` - HTTP Basic authentication
+* `digest` - HTTP Digest authentication; this prevents the password from being
+ transmitted to the proxy in clear text
+* `negotiate` - GSS-Negotiate authentication (compare the --negotiate option
+ of `curl(1)`)
+* `ntlm` - NTLM authentication (compare the --ntlm option of `curl(1)`)
+--
+
http.cookieFile::
File containing previously stored cookie lines which should be used
in the Git http session, if they match the server. The file format
@@ -2390,6 +2411,11 @@ remote..proxy::
the proxy to use for that remote. Set to the empty string to
disable proxying for that remote.
+remote..proxyAuthMethod::
+ For remotes that require curl (http, https and ftp), the method to use
for
+ authenticating against the proxy in use (probably set in
+ `remote..proxy`). See `http.proxyAuthMethod`.
+
remote..fetch::
The default set of "refspec" for linkgit:git-fetch[1]. See
linkgit:git-fetch[1].
diff --git a/Documentation/technical/api-remote.txt
b/Documentation/technical/api-remote.txt
index 2cfdd22..f10941b 100644
--- a/Documentation/technical/api-remote.txt
+++ b/Documentation/technical/api-remote.txt
@@ -51,6 +51,10 @@ struct remote
The proxy to use for curl (http, https, ftp, etc.) URLs.
+`http_proxy_authmethod`::
+
+ The method used for authenticating against `http_proxy`.
+
struct remotes can be found by name with remote_get(), and iterated
through with for_each_remote(). remote_get(NULL) will return the
default remote, given the current branch and configuration.
diff --git a/http.c b/http.c
index 7da76ed..a786802 100644
--- a/http.c
+++ b/http.c
@@ -63,6 +63,24 @@ 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 *http_proxy_authmethod;
+static struct {
+ const char *name;
+ long curlauth_param;
+} proxy_authmethods[] = {
+ { "basic", CURLAUTH_BASIC },
+ { "digest", CURLAUTH_DIGEST },
+ { "negotiate", CURLAUTH_GSSNEGOTIATE },
+ { "ntlm", CURLAUTH_NTLM },
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+ { "anyauth", CURLAUTH_ANY },
+#endif
+ /*
+* CURLAUTH_DIGEST_IE has no corresponding command-line option in
+* curl(1) and is not included in CURLAUTH_ANY, so we leave it out
+* here, too
+*/
+};
static const char *curl_cookie_file;
static int curl_save_cookies;
struct credential http_auth = CREDENTIAL_INIT;
@@ -257,6 +275,9 @@ static int http_options(const char *var, const char *value,
void *cb)
if (!strcmp("http.proxy", var))
return git_config_string(&curl_http_proxy, var, value);
+ if (!strcmp("http.proxyauthmethod", var))
+ return git_config_string(&http_proxy_authmethod, var, value);
+
if (!strcmp("http.cookiefile&q
Re: [PATCH 2/2] http: use credential API to handle proxy authentication
On 2015-11-02 14:54, Junio C Hamano wrote:
> > 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.
I've actually tested this with CURL 7.15.5 (0x070f05), and didn't get any
compilation error.
Cheers,
Knut
--
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Dr. Arno Steitz
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] http: allow selection of proxy authentication method
On 2015-11-02 14:46, Junio C Hamano wrote:
> > Reviewed-by: Junio C Hamano
> > Reviewed-by: Eric Sunshine
>
> Please add these only when you are doing the final submission,
> sending the same version reviewed by these people after they said
> the patch(es) look good. To credit others for helping you to polish
> your patch, Helped-by: would be more appropriate.
Sorry about that.
However, may I suggest that Documentation/SubmittingPatches could do with a
little rewording in this respect?
> Do not forget to add trailers such as "Acked-by:", "Reviewed-by:" and
> "Tested-by:" lines as necessary to credit people who helped your
> patch.
"Helped-by:" isn't even mentioned.
> > +static void init_curl_proxy_auth(CURL *result)
> > +{
> > + env_override(&http_proxy_authmethod, "GIT_HTTP_PROXY_AUTHMETHOD");
>
> Shouldn't this also be part of the #if/#endif?
The idea here was to have as little code as possible within the #if/#endif, as a
matter of principle. It may be a little construed in this case, but supposing
there's some subtle bug with env_override, or a future change introduces one,
having it occur only for certain CURL versions would tend to make it harder to
track down.
> and this code would be:
>
> if (remote)
> var_override(&http_proxy_authmethod,
> remote->http_proxy_authmethod);
Good catch.
Cheers,
Knut
--
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Dr. Arno Steitz
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] http: use credential API to handle proxy authentication
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
Reviewed-by: Junio C Hamano
Reviewed-by: Eric Sunshine
---
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;
static struct {
const char *name;
@@ -81,6 +81,8 @@ static struct {
* here, too
*/
};
+static struct credential proxy_auth = CREDENTIAL_INIT;
+static const char *curl_proxyuserpwd = NULL;
static const char *curl_cookie_file;
static int curl_save_cookies;
struct credential http_auth = CREDENTIAL_INIT;
@@ -178,6 +180,9 @@ static void finish_active_slot(struct active_request_slot
*slot)
#else
slot->results->auth_avail = 0;
#endif
+
+ curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CONNECTCODE,
+ &slot->results->http_connectcode);
}
/* Run callback if appropriate */
@@ -339,6 +344,24 @@ static void env_override(const char **var, const char
*envname)
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
+ }
+
env_override(&http_proxy_authmethod, "GIT_HTTP_PROXY_AUTHMETHOD");
#if LIBCURL_VERSION_NUM >= 0x070a07 /* CURLOPT_PROXYAUTH and CURLAUTH_ANY */
@@ -520,8 +543,42 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_USE_SSL, CURLUSESSL_TRY);
#endif
+ /*
+* CURL also examines these variables as a fallback; but we need to
query
+* them here in order to decide whether to prompt for missing password
(cf.
+* init_curl_proxy_auth()).
+*
+* Unlike many other common environment variables, these are
historically
+* lowercase only. It appears that CURL did not know this and
implemented
+* only uppercase variants, which was later corrected to take both -
with
+* the exception of http_proxy, which is lowercase only also in CURL. As
+* the lowercase versions are the historical quasi-standard, they take
+* precedence here, as in CURL.
+*/
+ if (!curl_http_proxy) {
+ if (!strcmp(http_auth.protocol, "https")) {
+ env_override(&curl_http_proxy, "HTTPS_PROXY");
+ env_override(&curl_http_proxy, "https_proxy");
+ } else {
+ env_override(&curl_http_proxy, "http_proxy");
+ }
+ if (!curl_http_proxy) {
+ env_override(&curl_http_proxy, "ALL_PROXY");
+ env_override(&curl_http_proxy, "all_proxy");
+ }
+ }
+
if (curl_http_proxy) {
- curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
+ if (strstr(curl_http_proxy, "://"))
+ credential_from_url(&proxy_auth, curl_http_proxy);
+ else {
+ struct s
[PATCH 1/2] http: allow selection of proxy authentication method
CURLAUTH_ANY does not work with proxies which answer unauthenticated requests
with a 307 redirect to an error page instead of a 407 listing supported
authentication methods. Therefore, allow the authentication method to be set
using the environment variable GIT_HTTP_PROXY_AUTHMETHOD or configuration
variables http.proxyAuthmethod and remote..proxyAuthmethod (in analogy
to http.proxy and remote..proxy).
The following values are supported:
* anyauth (default)
* basic
* digest
* negotiate
* ntlm
Signed-off-by: Knut Franke
Reviewed-by: Junio C Hamano
Reviewed-by: Eric Sunshine
---
Documentation/config.txt | 26 +
http.c | 72 ++--
remote.c | 3 ++
remote.h | 1 +
4 files changed, 99 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 391a0c3..6f29893 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1597,6 +1597,27 @@ http.proxy::
`curl(1)`). This can be overridden on a per-remote basis; see
remote..proxy
+http.proxyAuthMethod::
+ Set the method with which to authenticate against the HTTP proxy. This
only
+ takes effect if the configured proxy URI contains a user name part
(i.e. is
+ of the form 'user@host' or 'user@host:port'). This can be overridden on
a
+ per-remote basis; see `remote..proxyAuthMethod`. Both can be
+ overridden by the 'GIT_HTTP_PROXY_AUTHMETHOD' environment variable.
+ Possible values are:
++
+--
+* `anyauth` - Automatically pick a suitable authentication method. It is
+ assumed that the proxy answers an unauthenticated request with a 407
+ status code and one or more Proxy-authenticate headers with supported
+ authentication methods. This is the default.
+* `basic` - HTTP Basic authentication
+* `digest` - HTTP Digest authentication; this prevents the password from being
+ transmitted to the proxy in clear text
+* `negotiate` - GSS-Negotiate authentication (compare the --negotiate option
+ of `curl(1)`)
+* `ntlm` - NTLM authentication (compare the --ntlm option of `curl(1)`)
+--
+
http.cookieFile::
File containing previously stored cookie lines which should be used
in the Git http session, if they match the server. The file format
@@ -2390,6 +2411,11 @@ remote..proxy::
the proxy to use for that remote. Set to the empty string to
disable proxying for that remote.
+remote..proxyAuthMethod::
+ For remotes that require curl (http, https and ftp), the method to use
for
+ authenticating against the proxy in use (probably set in
+ `remote..proxy`). See `http.proxyAuthMethod`.
+
remote..fetch::
The default set of "refspec" for linkgit:git-fetch[1]. See
linkgit:git-fetch[1].
diff --git a/http.c b/http.c
index 7da76ed..1172819 100644
--- a/http.c
+++ b/http.c
@@ -63,6 +63,24 @@ 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 *http_proxy_authmethod = NULL;
+static struct {
+ const char *name;
+ long curlauth_param;
+} proxy_authmethods[] = {
+ { "basic", CURLAUTH_BASIC },
+ { "digest", CURLAUTH_DIGEST },
+ { "negotiate", CURLAUTH_GSSNEGOTIATE },
+ { "ntlm", CURLAUTH_NTLM },
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+ { "anyauth", CURLAUTH_ANY },
+#endif
+ /*
+* CURLAUTH_DIGEST_IE has no corresponding command-line option in
+* curl(1) and is not included in CURLAUTH_ANY, so we leave it out
+* here, too
+*/
+};
static const char *curl_cookie_file;
static int curl_save_cookies;
struct credential http_auth = CREDENTIAL_INIT;
@@ -257,6 +275,9 @@ static int http_options(const char *var, const char *value,
void *cb)
if (!strcmp("http.proxy", var))
return git_config_string(&curl_http_proxy, var, value);
+ if (!strcmp("http.proxyauthmethod", var))
+ return git_config_string(&http_proxy_authmethod, var, value);
+
if (!strcmp("http.cookiefile", var))
return git_config_string(&curl_cookie_file, var, value);
if (!strcmp("http.savecookies", var)) {
@@ -305,6 +326,42 @@ static void init_curl_http_auth(CURL *result)
#endif
}
+/* assumes *var is either NULL or free-able */
+static void env_override(const char **var, const char *envname)
+{
+ const char *val = getenv(envname);
+ if (val) {
+ if (*var)
+ free((void*)*var);
+ *var = xstrdup(val);
+ }
+}
+
+static void init_curl_proxy_auth(CURL *result)
+{
+ env_override(&http_proxy_authmethod, "GIT_HTTP_PROXY_AUTHMETHOD");
+
+#if LIBCURL_VERSION_NUM
[PATCH v3 0/2]
Changes in the third iteration: * don't break support for curl < 7.10.7 * fix some memory leaks * explicitly set anyauth as fallback for unsupported proyx authmethod setting, and tell the user what we did * clean up usage of curl version #ifdefs * fix more code formatting / style / naming issues * add in-code comment explaining the proxy variable uppercase/lowercase mess -- Vorstandsvorsitzender/Chairman of the board of management: Gerd-Lothar Leonhart Vorstand/Board of Management: Dr. Bernd Finkbeiner, Dr. Arno Steitz Vorsitzender des Aufsichtsrats/ Chairman of the Supervisory Board: Philippe Miltin Sitz/Registered Office: Tuebingen Registergericht/Registration Court: Stuttgart Registernummer/Commercial Register No.: HRB 382196 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] http: use credential API to handle proxy authentication
On 2015-10-28 14:58, Eric Sunshine wrote:
> > + }
> > + if (!curl_http_proxy) {
> > + copy_from_env(&curl_http_proxy, "ALL_PROXY");
> > + copy_from_env(&curl_http_proxy, "all_proxy");
> > + }
>
> If this sort of upper- and lowercase environment variable name
> checking is indeed desirable, I wonder if it would make sense to fold
> that functionality into the helper function.
It's just for consistency with libcurl here, not generally desirable; so I don't
think it makes sense to add it to the helper.
Otherwise, will fix. Thanks.
Cheers,
Knut
--
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Dr. Arno Steitz
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] http: allow selection of proxy authentication method
Junio C Hamano wrote:
> > + if (http_proxy_authmethod) {
> > + int i;
> > + for (i = 0; i < ARRAY_SIZE(http_proxy_authmethods); i++) {
> > + if (!strcmp(http_proxy_authmethod,
> > http_proxy_authmethods[i].name)) {
> > + curl_easy_setopt(result, CURLOPT_PROXYAUTH,
> > +
> > http_proxy_authmethods[i].curlauth_param);
> > + break;
> > + }
> > + }
> > + if (i == ARRAY_SIZE(http_proxy_authmethods)) {
> > + warning("unsupported proxy authentication method %s:
> > using default",
> > + http_proxy_authmethod);
> > + }
> > + }
> > +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
> > + else
> > + curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
> > +#endif
> > +}
>
> This patch should take what 1c2dbf20 (http: support curl < 7.10.7,
> 2015-02-03) wanted to do into account. Having the configuration
> variable or the environment variable defined by itself, while
> running a Git built with old cURL, shouldn't trigger any warning,
> but the entire function should perhaps be ifdefed out or something?
Maybe add a #define LIBCURL_CAN_HANDLE_PROXY_AUTH to clarify this, like we do
with LIBCURL_CAN_HANDLE_AUTH_ANY? If so, should this be a separate patch? With
the current (scattered) version dependencies, it took me a while to realize that
if the function is ifdefed out for LIBCURL_VERSION_NUM < 0x070a07, we don't need
to worry about default behavior in case LIBCURL_CAN_HANDLE_AUTH_ANY is not
defined. (on the other hand, looking at other curl-version-ifdefs, the define
for AUTH_ANY is the exception)
> >> +static void copy_from_env(const char **var, const char *envname)
> >> +{
> >> + const char *val = getenv(envname);
> >> + if (val)
> >> + *var = xstrdup(val);
> >> +}
[...]
> I primarily was
> wishing that its name more clearly conveyed that it sets the
> variable from the environment _only if_ the environment variable
> exists, and otherwise it does not clobber.
How about env_override? Not perfect, but probably better. I don't think
squeezing in more information (maybe_env_override, override_from_env_var) would
help.
> The implementation of the helper seems to assume that the variable
> must not be pointing at a free-able piece of memory when it is
> called
In fact, if http.proxyAuthMethod (btw, I agree with Eric about capitalization)
is set, I do call it with *var pointing to free-able memory. Will fix this.
FWIW, set_from_env() has the same pre-condition, which doesn't seem to be
satisfied in all cases (namely when overwriting variables previously set by
git_config_string()); not to mention missing free()s in http_cleanup.
Otherwise, I'll make the suggested fixes. Thanks.
Cheers,
Knut
--
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Dr. Arno Steitz
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] http proxy authentication improvements
Fixes in the second iteration: * rename http.proxy-authmethod to http.proxyAuthmethod for consistency with other core git variables * issue warning() instead of error() for unsupported authentication method, for consistency with http.sslVersion * fix some code formatting / style issues * fix memory management bug -- Vorstandsvorsitzender/Chairman of the board of management: Gerd-Lothar Leonhart Vorstand/Board of Management: Dr. Bernd Finkbeiner, Dr. Arno Steitz Vorsitzender des Aufsichtsrats/ Chairman of the Supervisory Board: Philippe Miltin Sitz/Registered Office: Tuebingen Registergericht/Registration Court: Stuttgart Registernummer/Commercial Register No.: HRB 382196 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] http: use credential API to handle proxy authentication
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
---
http.c | 63 ++-
http.h | 1 +
2 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/http.c b/http.c
index 4756bab..11bebe1 100644
--- a/http.c
+++ b/http.c
@@ -79,6 +79,7 @@ static struct {
// curl(1) and is not included in CURLAUTH_ANY, so we leave it out
// here, too
};
+struct credential http_proxy_auth = CREDENTIAL_INIT;
static const char *curl_cookie_file;
static int curl_save_cookies;
struct credential http_auth = CREDENTIAL_INIT;
@@ -176,6 +177,9 @@ static void finish_active_slot(struct active_request_slot
*slot)
#else
slot->results->auth_avail = 0;
#endif
+
+ curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CONNECTCODE,
+ &slot->results->http_connectcode);
}
/* Run callback if appropriate */
@@ -333,6 +337,25 @@ static void copy_from_env(const char **var, const char
*envname)
static void init_curl_proxy_auth(CURL *result)
{
+ if (http_proxy_auth.username) {
+ if (!http_proxy_auth.password) {
+ credential_fill(&http_proxy_auth);
+ }
+#if LIBCURL_VERSION_NUM >= 0x071301
+ curl_easy_setopt(result, CURLOPT_PROXYUSERNAME,
+ http_proxy_auth.username);
+ curl_easy_setopt(result, CURLOPT_PROXYPASSWORD,
+ http_proxy_auth.password);
+#else
+ struct strbuf up = STRBUF_INIT;
+ strbuf_reset(&up);
+ strbuf_addstr_urlencode(&up, http_proxy_auth.username, 1);
+ strbuf_addch(&up, ':');
+ strbuf_addstr_urlencode(&up, http_proxy_auth.password, 1);
+ curl_easy_setopt(result, CURLOPT_PROXYUSERPWD, up.buf);
+#endif
+ }
+
copy_from_env(&http_proxy_authmethod, "GIT_HTTP_PROXY_AUTHMETHOD");
if (http_proxy_authmethod) {
@@ -513,8 +536,36 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_USE_SSL, CURLUSESSL_TRY);
#endif
+ /*
+* curl also examines these variables as a fallback; but we need to
query
+* them here in order to decide whether to prompt for missing password
(cf.
+* init_curl_proxy_auth()).
+*/
+ if (!curl_http_proxy) {
+ if (!strcmp(http_auth.protocol, "https")) {
+ copy_from_env(&curl_http_proxy, "HTTPS_PROXY");
+ copy_from_env(&curl_http_proxy, "https_proxy");
+ } else {
+ copy_from_env(&curl_http_proxy, "http_proxy");
+ }
+ if (!curl_http_proxy) {
+ copy_from_env(&curl_http_proxy, "ALL_PROXY");
+ copy_from_env(&curl_http_proxy, "all_proxy");
+ }
+ }
+
if (curl_http_proxy) {
- curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
+ if (strstr(curl_http_proxy, "://"))
+ credential_from_url(&http_proxy_auth, curl_http_proxy);
+ else {
+ struct strbuf url = STRBUF_INIT;
+ strbuf_reset(&url);
+ strbuf_addstr(&url, "http://";);
+ strbuf_addstr(&url, curl_http_proxy);
+ credential_from_url(&http_proxy_auth, url.buf);
+ }
+
+ curl_easy_setopt(result, CURLOPT_PROXY, http_proxy_auth.host);
}
init_curl_proxy_auth(result);
@@ -658,6 +709,12 @@ void http_cleanup(void)
curl_http_proxy = NULL;
}
+ if (http_proxy_auth.password) {
+ memset(http_proxy_auth.password, 0,
strlen(http_proxy_auth.password));
+ free(http_proxy_auth.password);
+ http_proxy_auth.password = NULL;
+ }
+
if (http_proxy_authmethod) {
free((void *)http_proxy_
[PATCH 1/2] http: allow selection of proxy authentication method
CURLAUTH_ANY does not work with proxies which answer unauthenticated requests
with a 307 redirect to an error page instead of a 407 listing supported
authentication methods. Therefore, allow the authentication method to be set
using the environment variable GIT_HTTP_PROXY_AUTHMETHOD or configuration
variables http.proxyAuthmethod and remote..proxyAuthmethod (in analogy
to http.proxy and remote..proxy).
The following values are supported:
* anyauth (default)
* basic
* digest
* negotiate
* ntlm
Signed-off-by: Knut Franke
---
Documentation/config.txt | 28 ++
http.c | 62 +---
remote.c | 3 +++
remote.h | 1 +
4 files changed, 91 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 391a0c3..f2644d1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1597,6 +1597,29 @@ http.proxy::
`curl(1)`). This can be overridden on a per-remote basis; see
remote..proxy
+http.proxyAuthmethod::
+ Set the method with which to authenticate against the HTTP proxy. This
only
+takes effect if the configured proxy URI contains a user name part (i.e. is
+of the form 'user@host' or 'user@host:port'). This can be overridden on a
+per-remote basis; see `remote..proxyAuthmethod`. Both can be
+overridden by the 'GIT_HTTP_PROXY_AUTHMETHOD' environment variable.
+Possible values are:
++
+--
+* `anyauth` - Automatically pick a suitable authentication method. It is
+ assumed that the proxy answers an unauthenticated request with a 407
+ status code and one or more Proxy-authenticate headers with supported
+ authentication methods. This is the default.
+* `basic` - HTTP Basic authentication
+* `digest` - HTTP Digest authentication; this prevents the password from being
+ transmitted to the proxy in clear text
+* `negotiate` - GSS-Negotiate authentication (compare the --negotiate option
+ of `curl(1)`)
+* `ntlm` - NTLM authentication (compare the --ntlm option of `curl(1)`)
+--
++
+
+
http.cookieFile::
File containing previously stored cookie lines which should be used
in the Git http session, if they match the server. The file format
@@ -2390,6 +2413,11 @@ remote..proxy::
the proxy to use for that remote. Set to the empty string to
disable proxying for that remote.
+remote..proxyAuthmethod::
+For remotes that require curl (http, https and ftp), the method to use for
+authenticating against the proxy in use (probably set in
+`remote..proxy`). See `http.proxyAuthmethod`.
+
remote..fetch::
The default set of "refspec" for linkgit:git-fetch[1]. See
linkgit:git-fetch[1].
diff --git a/http.c b/http.c
index 7da76ed..4756bab 100644
--- a/http.c
+++ b/http.c
@@ -63,6 +63,22 @@ 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 *http_proxy_authmethod = NULL;
+static struct {
+ const char *name;
+ long curlauth_param;
+} http_proxy_authmethods[] = {
+ { "basic", CURLAUTH_BASIC },
+ { "digest", CURLAUTH_DIGEST },
+ { "negotiate", CURLAUTH_GSSNEGOTIATE },
+ { "ntlm", CURLAUTH_NTLM },
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+ { "anyauth", CURLAUTH_ANY },
+#endif
+ // CURLAUTH_DIGEST_IE has no corresponding command-line option in
+ // curl(1) and is not included in CURLAUTH_ANY, so we leave it out
+ // here, too
+};
static const char *curl_cookie_file;
static int curl_save_cookies;
struct credential http_auth = CREDENTIAL_INIT;
@@ -257,6 +273,9 @@ static int http_options(const char *var, const char *value,
void *cb)
if (!strcmp("http.proxy", var))
return git_config_string(&curl_http_proxy, var, value);
+ if (!strcmp("http.proxyauthmethod", var))
+ return git_config_string(&http_proxy_authmethod, var, value);
+
if (!strcmp("http.cookiefile", var))
return git_config_string(&curl_cookie_file, var, value);
if (!strcmp("http.savecookies", var)) {
@@ -305,6 +324,37 @@ static void init_curl_http_auth(CURL *result)
#endif
}
+static void copy_from_env(const char **var, const char *envname)
+{
+ const char *val = getenv(envname);
+ if (val)
+ *var = xstrdup(val);
+}
+
+static void init_curl_proxy_auth(CURL *result)
+{
+ copy_from_env(&http_proxy_authmethod, "GIT_HTTP_PROXY_AUTHMETHOD");
+
+ if (http_proxy_authmethod) {
+ int i;
+ for (i = 0; i < ARRAY_SIZE(http_proxy_authmethods); i++) {
+ if (!strcmp(http_proxy_authmethod,
http_proxy_authmethods[i].name)) {
+
Re: [PATCH 1/2] http: allow selection of proxy authentication method
On 2015-10-26 13:33, Junio C Hamano wrote:
> Call yours "http.proxyAuthmethod" in the documentation, and use
> strcmp("http.proxyauthmethod", var) in the options callback code.
[...]
> Strange indentation here...
[...]
> Along the same line as how we do sslversions[] instead of a long
> if/else if/ chain is preferrable.
Thanks for the feedback, I'll fix these in the next iteration.
Cheers,
Knut
--
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Dr. Arno Steitz
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] http: allow selection of proxy authentication method
CURLAUTH_ANY does not work with proxies which answer unauthenticated requests
with a 307 redirect to an error page instead of a 407 listing supported
authentication methods. Therefore, allow the authentication method to be set
using the environment variable GIT_HTTP_PROXY_AUTHMETHOD or configuration
variables http.proxy-authmethod and remote..proxy-authmethod (in analogy
to http.proxy and remote..proxy).
The following values are supported:
* anyauth (default)
* basic
* digest
* negotiate
* ntlm
Signed-off-by: Knut Franke
---
Documentation/config.txt | 28 ++
http.c | 61
remote.c | 3 +++
remote.h | 1 +
4 files changed, 83 insertions(+), 10 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 391a0c3..9dff88d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1597,6 +1597,29 @@ http.proxy::
`curl(1)`). This can be overridden on a per-remote basis; see
remote..proxy
+http.proxy-authmethod::
+ Set the method with which to authenticate against the HTTP proxy. This
only
+takes effect if the configured proxy URI contains a user name part (i.e. is
+of the form 'user@host' or 'user@host:port'). This can be overridden on a
+per-remote basis; see `remote..proxy-authmethod`. Both can be
+overridden by the 'GIT_HTTP_PROXY_AUTHMETHOD' environment variable.
+Possible values are:
++
+--
+* `anyauth` - Automatically pick a suitable authentication method. It is
+ assumed that the proxy answers an unauthenticated request with a 407
+ status code and one or more Proxy-authenticate headers with supported
+ authentication methods. This is the default.
+* `basic` - HTTP Basic authentication
+* `digest` - HTTP Digest authentication; this prevents the password from being
+ transmitted to the proxy in clear text
+* `negotiate` - GSS-Negotiate authentication (compare the --negotiate option
+ of `curl(1)`)
+* `ntlm` - NTLM authentication (compare the --ntlm option of `curl(1)`)
+--
++
+
+
http.cookieFile::
File containing previously stored cookie lines which should be used
in the Git http session, if they match the server. The file format
@@ -2390,6 +2413,11 @@ remote..proxy::
the proxy to use for that remote. Set to the empty string to
disable proxying for that remote.
+remote..proxy-authmethod::
+For remotes that require curl (http, https and ftp), the method to use for
+authenticating against the proxy in use (probably set in
+`remote..proxy`). See `http.proxy-authmethod`.
+
remote..fetch::
The default set of "refspec" for linkgit:git-fetch[1]. See
linkgit:git-fetch[1].
diff --git a/http.c b/http.c
index 7da76ed..f7366d0 100644
--- a/http.c
+++ b/http.c
@@ -63,6 +63,7 @@ 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 *http_proxy_authmethod = NULL;
static const char *curl_cookie_file;
static int curl_save_cookies;
struct credential http_auth = CREDENTIAL_INIT;
@@ -257,6 +258,9 @@ static int http_options(const char *var, const char *value,
void *cb)
if (!strcmp("http.proxy", var))
return git_config_string(&curl_http_proxy, var, value);
+ if (!strcmp("http.proxy-authmethod", var))
+ return git_config_string(&http_proxy_authmethod, var, value);
+
if (!strcmp("http.cookiefile", var))
return git_config_string(&curl_cookie_file, var, value);
if (!strcmp("http.savecookies", var)) {
@@ -305,6 +309,43 @@ static void init_curl_http_auth(CURL *result)
#endif
}
+static void set_from_env(const char **var, const char *envname)
+{
+ const char *val = getenv(envname);
+ if (val)
+ *var = val;
+}
+
+static void init_curl_proxy_auth(CURL *result)
+{
+set_from_env(&http_proxy_authmethod, "GIT_HTTP_PROXY_AUTHMETHOD");
+
+ if (http_proxy_authmethod) {
+ if (!strcmp(http_proxy_authmethod, "basic"))
+ curl_easy_setopt(result, CURLOPT_PROXYAUTH,
CURLAUTH_BASIC);
+ else if (!strcmp(http_proxy_authmethod, "digest"))
+ curl_easy_setopt(result, CURLOPT_PROXYAUTH,
CURLAUTH_DIGEST);
+ else if (!strcmp(http_proxy_authmethod, "negotiate"))
+ curl_easy_setopt(result, CURLOPT_PROXYAUTH,
CURLAUTH_GSSNEGOTIATE);
+ else if (!strcmp(http_proxy_authmethod, "ntlm"))
+ curl_easy_setopt(result, CURLOPT_PROXYAUTH,
CURLAUTH_NTLM);
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+ else if (!strcmp(http_proxy_authmethod, "anyauth"))
+
[PATCH 2/2] http: use credential API to handle proxy authentication
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
---
http.c | 63 ++-
http.h | 1 +
2 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/http.c b/http.c
index f7366d0..6e20017 100644
--- a/http.c
+++ b/http.c
@@ -64,6 +64,7 @@ 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 = NULL;
+struct credential http_proxy_auth = CREDENTIAL_INIT;
static const char *curl_cookie_file;
static int curl_save_cookies;
struct credential http_auth = CREDENTIAL_INIT;
@@ -161,6 +162,9 @@ static void finish_active_slot(struct active_request_slot
*slot)
#else
slot->results->auth_avail = 0;
#endif
+
+ curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CONNECTCODE,
+ &slot->results->http_connectcode);
}
/* Run callback if appropriate */
@@ -318,6 +322,25 @@ static void set_from_env(const char **var, const char
*envname)
static void init_curl_proxy_auth(CURL *result)
{
+ if (http_proxy_auth.username) {
+ if (!http_proxy_auth.password) {
+ credential_fill(&http_proxy_auth);
+ }
+#if LIBCURL_VERSION_NUM >= 0x071301
+ curl_easy_setopt(result, CURLOPT_PROXYUSERNAME,
+ http_proxy_auth.username);
+ curl_easy_setopt(result, CURLOPT_PROXYPASSWORD,
+ http_proxy_auth.password);
+#else
+ struct strbuf up = STRBUF_INIT;
+ strbuf_reset(&up);
+ strbuf_addstr_urlencode(&up, http_proxy_auth.username, 1);
+ strbuf_addch(&up, ':');
+ strbuf_addstr_urlencode(&up, http_proxy_auth.password, 1);
+ curl_easy_setopt(result, CURLOPT_PROXYUSERPWD, up.buf);
+#endif
+ }
+
set_from_env(&http_proxy_authmethod, "GIT_HTTP_PROXY_AUTHMETHOD");
if (http_proxy_authmethod) {
@@ -505,8 +528,36 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_USE_SSL, CURLUSESSL_TRY);
#endif
+ /*
+* curl also examines these variables as a fallback; but we need to
query
+* them here in order to decide whether to prompt for missing password
(cf.
+* init_curl_proxy_auth()).
+*/
+ if (!curl_http_proxy) {
+ if (!strcmp(http_auth.protocol, "https")) {
+ set_from_env(&curl_http_proxy, "HTTPS_PROXY");
+ set_from_env(&curl_http_proxy, "https_proxy");
+ } else {
+ set_from_env(&curl_http_proxy, "http_proxy");
+ }
+ if (!curl_http_proxy) {
+ set_from_env(&curl_http_proxy, "ALL_PROXY");
+ set_from_env(&curl_http_proxy, "all_proxy");
+ }
+ }
+
if (curl_http_proxy) {
- curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
+ if (strstr(curl_http_proxy, "://"))
+ credential_from_url(&http_proxy_auth, curl_http_proxy);
+ else {
+ struct strbuf url = STRBUF_INIT;
+ strbuf_reset(&url);
+ strbuf_addstr(&url, "http://";);
+ strbuf_addstr(&url, curl_http_proxy);
+ credential_from_url(&http_proxy_auth, url.buf);
+ }
+
+ curl_easy_setopt(result, CURLOPT_PROXY, http_proxy_auth.host);
}
init_curl_proxy_auth(result);
@@ -643,6 +694,12 @@ void http_cleanup(void)
curl_http_proxy = NULL;
}
+ if (http_proxy_auth.password) {
+ memset(http_proxy_auth.password, 0,
strlen(http_proxy_auth.password));
+ free(http_proxy_auth.password);
+ http_proxy_auth.password = NULL;
+ }
+
if (http_proxy_authmethod) {
[PATCH] gitk: Add menu item for reverting commits
Sometimes it's helpful (at least psychologically) to have this feature
easily accessible. Code borrows heavily from cherrypick.
Signed-off-by: Knut Franke
---
gitk | 62
++
1 file changed, 62 insertions(+)
diff --git a/gitk b/gitk
index 572f73f..fb1a6ce 100755
--- a/gitk
+++ b/gitk
@@ -2562,6 +2562,7 @@ proc makewindow {} {
{mc "Compare with marked commit" command compare_commits}
{mc "Diff this -> marked commit" command {diffvsmark 0}}
{mc "Diff marked commit -> this" command {diffvsmark 1}}
+ {mc "Revert this commit" command revert}
}
$rowctxmenu configure -tearoff 0
@@ -9347,6 +9348,67 @@ proc cherrypick {} {
notbusy cherrypick
}
+proc revert {} {
+global rowmenuid curview
+global mainhead mainheadid
+global gitdir
+
+set oldhead [exec git rev-parse HEAD]
+set dheads [descheads $rowmenuid]
+if { $dheads eq {} || [lsearch -exact $dheads $oldhead] == -1 } {
+ set ok [confirm_popup [mc "Commit %s is not\
+ included in branch %s -- really revert it?" \
+ [string range $rowmenuid 0 7] $mainhead]]
+ if {!$ok} return
+}
+nowbusy revert [mc "Reverting"]
+update
+
+if [catch {exec git revert --no-edit $rowmenuid} err] {
+notbusy revert
+if [regexp {files would be overwritten by merge:(\n(( |\t)+[^
\n]+\n)+)}\
+$err match files] {
+regsub {\n( |\t)+} $files "\n" files
+error_popup [mc "Revert failed because of local changes to
\
+the following files:%s Please commit, reset or stash \
+your changes and try again." $files]
+} elseif [regexp {error: could not revert} $err] {
+if [confirm_popup [mc "Revert failed because of merge
conflict.\n\
+Do you wish to run git citool to resolve it?"]] {
+# Force citool to read MERGE_MSG
+file delete [file join $gitdir "GITGUI_MSG"]
+exec_citool {} $rowmenuid
+}
+} else { error_popup $err }
+run updatecommits
+return
+}
+
+set newhead [exec git rev-parse HEAD]
+if { $newhead eq $oldhead } {
+notbusy revert
+error_popup [mc "No changes committed"]
+return
+}
+
+addnewchild $newhead $oldhead
+
+if [commitinview $oldhead $curview] {
+# XXX this isn't right if we have a path limit...
+insertrow $newhead $oldhead $curview
+if {$mainhead ne {}} {
+movehead $newhead $mainhead
+movedhead $newhead $mainhead
+}
+set mainheadid $newhead
+redrawtags $oldhead
+redrawtags $newhead
+selbyid $newhead
+}
+
+notbusy revert
+}
+
proc resethead {} {
global mainhead rowmenuid confirm_ok resettype NS
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
