[PATCH] Fix https interactive authentication problem

[PATCH] Fix https interactive authentication problem

[Steinmann Ruedi]

>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(-)

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

Re: [PATCH] Fix https interactive authentication problem

[Jeff King]

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

> 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

Re: [PATCH] Fix https interactive authentication problem

[Ruedi Steinmann]

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