threads / patch / 29004

patchFix https interactive authentication problem

Subject: [PATCH] Fix https interactive authentication problem

## tl;dr

3 messages between Nov 23, 2011 and Nov 25, 2011. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Steinmann Ruedi· Nov 23, 2011, 07:58 UTC · lore

>From 986e29085ee2215a3e0a412ee7874dc2d0ef36be Mon Sep 17 00:00:00 2001

From: Ruedi Steinmann <ruediste@student.ethz.ch>
Date: Wed, 23 Nov 2011 08:41:52 +0100
Subject: [PATCH] Fix https interactive authentication problem

Cloning a repository over https works fine when the username/password is given in the URL. But if it is queried interactively, an error occurs(see below). I found that the username/password is not set when a connection is reused.

With this patch, the username/password is set whenever a connection is reused.

Sample output showing the error:

git clone https://n.ethz.ch/student/... Cloning into ... Username: Password: error: Unable to get pack file https://n.ethz.ch/student/.../objects/pack/pack-1ced2ebff0c9fc1f07e0c7cc9dd3fc75f6ac6962.pack The requested URL returned error: 401 ... error: Fetch failed.

Signed-off-by: Ruedi Steinmann <ruediste@student.ethz.ch>
---
 http.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

Show changes to http.c +4 −2

diff --git a/http.c b/http.c
index e6c7597..c7b3558 100644
--- a/http.c
+++ b/http.c
@@ -550,7 +550,10 @@ struct active_request_slot *get_active_slot(void)
     curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, NULL);
     curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0);
     curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
-
+
+    // set username and password if already set
+    init_curl_http_auth(slot->curl);
+
     return slot;
 }

--
1.7.5.4

Jeff King· Nov 23, 2011, 22:51 UTC · re: Steinmann Ruedi · lore

Re: [PATCH] Fix https interactive authentication problem

On Wed, Nov 23, 2011 at 07:58:02AM +0000, Steinmann  Ruedi wrote:

Show 16 quoted lines

> Cloning a repository over https works fine when the username/password is
> given in the URL. But if it is queried interactively, an error occurs(see below).
> I found that the username/password is not set when a connection is reused.
> 
> With this patch, the username/password is set whenever a connection is reused.
> 
> Sample output showing the error:
> 
> git clone https://n.ethz.ch/student/...
> Cloning into ...
> Username:
> Password:
> error: Unable to get pack file https://n.ethz.ch/student/.../objects/pack/pack-1ced2ebff0c9fc1f07e0c7cc9dd3fc75f6ac6962.pack
> The requested URL returned error: 401
> ...
> error: Fetch failed.

Hmm. What version of git are you using?

We used to always set up the auth information in get_curl_handle, so all handles had any auth information that we already got. But we removed that recently in 986bbc0 (http: don't always prompt for password, 2011-11-04), which is in v1.7.8-rc1 and later.

I'm wondering if this was an unintended side effect of that patch.

Usually git will notice the 401 and retry with credentials (requesting them from the user if necessary). That used to not work for all requests but I fixed that in 8d677ed (http: retry authentication failures for all http requests, 2011-07-18), which is also in v1.7.8-rc*.

However, it looks like the packfile-fetching code for dumb http does not actually go through the usual http_request method, and does not recognize the 401.

So if you are using v1.7.8-rc*, then I think that is the issue. And your patch is just undoing what 8d677ed did. But the right fix is to refactor the packfile-fetching function on top of the other http-fetching code, so it can get the benefit of that logic.

If you are using an older version of git (pre-8d677ed), then one guess about what is happening is a race condition with setting up multiple curl handles:

  1. we get curl handle 1 from get_curl_handle, calling
     init_curl_http_auth. But we have no username in the url, so we
     assume no credentials are needed.

  2. we get curl handle 2 from get_curl_handle, as above.

  3. we make a request on handle one, get a 401, and ask for
     credentials.

  4. we make a request for a packfile on handle 2, but we never copied
     the auth from step (3) into the curl handle from step (2), and it
     fails.

(I haven't looked closely at our usage of curl_multi_*, so I am not 100% sure that this can even happen. So consider it just a theory).

If this is the case, your fix works because it calls init_curl_http_auth more often (i.e., before making any request, not just when we create the handle). But that means it is also undoing what 8d677ed did; we will ask the user for credentials before actually seeing a 401.

We could still fix this by teaching the packfile-fetching code to respect 401s. However, it does incur an unnecessary round-trip (we already know from the other request that credentials are required, but we don't bother to use them).

So I think the logic you want instead is "if we already have credentials, use them with this curl slot, but otherwise don't bother the user". Which would mean splitting the prompting bit out of init_curl_http_auth.

-Peff

Ruedi Steinmann· Nov 25, 2011, 12:10 UTC · re: Jeff King · lore

Re: [PATCH] Fix https interactive authentication problem

Jeff King <peff <at> peff.net> writes:

> 
> Hmm. What version of git are you using?
>

Hi Jeff,

I'm a little short of time at the moment, I'll come to this back later. So just the quick infos. I discovered the error in my preinstalled version of git (1.7.5.4). I reproduced the error with the version of git I have the source code of: (which I should have done before, sorry)

../git/git/git clone https://n.ethz.ch/... Cloning into 'masterthesis'... warning: templates not found /home/ruedi/share/git-core/templates Username for 'n.ethz.ch': Password for 'n.ethz.ch': error: The requested URL returned error: 401 (curl_result = 22, http_code = 401, sha1 = 4e69f4215f0d8c02ec1c29aadcd4ea39bb4e7c5e) error: Unable to find 4e69f4215f0d8c02ec1c29aadcd4ea39bb4e7c5e under https://n.ethz.ch/.../masterthesis.git Cannot obtain needed commit 4e69f4215f0d8c02ec1c29aadcd4ea39bb4e7c5e while processing commit 0fa41a9d4b3282891173ec54c04e0e3763867807. error: Fetch failed.

I am working with the git repository from https://github.com/gitster/git.git.

>From the first few lines of my commit from gitk:

Parent: 017d1e134545db0d162908f3538077eaa1f34fb6 (Update 1.7.8 draft release
notes in preparation for rc4)
Branch: master
Follows: v1.7.7.4, v1.7.8-rc3

Hope this helps

Cheers Ruedi