Re: git clone with basic auth in url directly returns authentication failure after 401 received under some git versions

From: Jeff King <peff@peff.net>
Date: Aug 22, 2022, 09:04 UTC
Message-ID: <YwNGoqcx3c27XpQL@coredump.intra.peff.net>
In-Reply-To: <o4sp3o6-75sp-o12o-2p29-r94s2s769r47@unkk.fr>

On Sun, Aug 21, 2022 at 12:32:54AM +0200, Daniel Stenberg wrote:

Show 5 quoted lines

> I would not mind having a discussion in the curl project to see if we should
> possibly consider adding a middle-ground where we allow sending credentials
> to another port for the same host name, but I am personally NOT sold on the
> idea. I think such redirects should rather be fixed and avoided - since I
> believe users will not understand the security implications of doing them.

I'm not 100% on it either. When it comes to security restrictions, sometimes simple-and-stupid is the best way. I was literally thinking of something as basic and restricted as:

  if (from_port == 80 && to_port == 443 &&
      from_protocol == HTTP && to_protocol == HTTPS)
        /* ok, allow it */

just because https upgrade is such a common (and presumably harmless) redirect. But possibly even that leaves wiggle room for bad things to happen. I'm happy to defer to you and other curl folks there.

I think the worst thing about the user experience from Git here is that it says "authentication failed", which is indistinguishable from a bogus credential. We don't even mention the redirect, because we don't warn about them unless the request ends up successful!

So I was thinking that curl could tell us "hey, I cleared the auth due to a redirect" via some curl_easy_getinfo() call. But maybe just noticing there was a redirect would be sufficient. This seems to work:

diff --git a/http.c b/http.c
index 5d0502f51f..0fe8a906e5 100644
--- a/http.c
+++ b/http.c
@@ -1934,7 +1934,7 @@ static int http_request_reauth(const char *url,
 {
 	int ret = http_request(url, result, target, options);
 
-	if (ret != HTTP_OK && ret != HTTP_REAUTH)
+	if (ret != HTTP_OK && ret != HTTP_REAUTH && ret != HTTP_NOAUTH)
 		return ret;
 
 	if (options && options->effective_url && options->base_url) {
diff --git a/remote-curl.c b/remote-curl.c
index b8758757ec..d462077a97 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -501,6 +501,12 @@ static struct discovery *discover_refs(const char *service, int for_push)
 		    transport_anonymize_url(url.buf));
 	case HTTP_NOAUTH:
 		show_http_message(&type, &charset, &buffer);
+		if (LIBCURL_VERSION_NUM >= 0x075300 &&
+		    strcmp(refs_url.buf, effective_url.buf)) {
+			char *u = transport_anonymize_url(url.buf);
+			warning(_("request redirected to %s"), u);
+			warning(_("authentication information may have been discarded"));
+		}
 		die(_("Authentication failed for '%s'"),
 		    transport_anonymize_url(url.buf));
 	case HTTP_NOMATCHPUBLICKEY:

Though I wonder if there is a cleaner way to determine what happened
than string comparisons.

-Peff