Re: git no longer prompting for password

2012-08-27 Thread Iain Paton
On 26/08/12 10:57, Iain Paton wrote:

 If %{THE_REQUEST} =~ /git-receive-pack/

I've just discovered that the If .. directive only appears in apache 2.4 
so something more generic will probably be a better idea. Not everyone will 
be running 2.4.x for a while yet.

Iain
--
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: git no longer prompting for password

2012-08-27 Thread BJ Hargrave
On Aug 27, 2012, at 04:28 , Iain Paton wrote:

 On 26/08/12 10:57, Iain Paton wrote:
 
If %{THE_REQUEST} =~ /git-receive-pack/
 
 I've just discovered that the If .. directive only appears in apache 2.4 
 so something more generic will probably be a better idea. Not everyone will 
 be running 2.4.x for a while yet.


You could try something like this:

Location /git
  # Require authentication for git push
  RewriteCond %{QUERY_STRING} service=git-receive-pack
  RewriteRule .* - [E=AUTHREQUIRED:yes]
  Order Allow,Deny
  Deny from env=AUTHREQUIRED
  Allow from all
  Satisfy Any
  # Whatever auth rules you want ...

I haven't tested this specific example but it is based upon similar rules I use 
on a 2.0 server to require auth when specific query parameters are present. In 
my case, I have the Rewrite rules in the VirtualHost and the other directives 
in the Directory being protected.
-- 

BJ



--
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: git no longer prompting for password

2012-08-26 Thread Iain Paton
On 25/08/12 21:39, Jeff King wrote:

 I think your regex is the culprit. The first request comes in with:
 
 GET /git/test.git/info/refs?service=git-receive-pack HTTP/1.1
 
 The odd URL is because we are probing to see if the server even supports
 smart-http. But note that it does not match your regex above, which
 requires /git-receive-pack. It looks like that is pulled straight from
 the git-http-backend manpage. I think the change in v1.7.8 broke people
 using that configuration.

Yes, it was lifted straight out of the manpage, albeit a couple of years 
ago now and there have been additions to the manpage since then. 
I did check, and the basic config is identical in the current manpage.

I can't be the only one using a config that's based on the example in 
the manpage surely ?  So I'm surprised this hasn't come up previously.


 I tend to think the right thing is to fix the configuration (both on
 your system and in the documentation), but we should probably also fix
 git to handle this situation more gracefully, since it used to work and
 has been advertised in the documentation for a long time.

So after some head scratching trying to work out how to do the equivalent of 
LocationMatch but on the query string I came up with the following:

ScriptAlias /git/ /usr/libexec/git-core/git-http-backend/

Directory /usr/libexec/git-core
Require ip 10.44.0.0/16
If %{THE_REQUEST} =~ /git-receive-pack/
AuthType Basic
AuthUserFile /data/git/htpasswd
AuthGroupfile /data/git/groups
AuthName Git Access

Require group committers
/If
/Directory

and I've removed the LocationMatch section completely.

So for accesses to git-http-backend I require auth if anything in the request 
includes git-receive-pack and that causes a prompt for the username/password 
as required, while at the same time it still allows anonymous pull.

It appears that the clone operation uses

GET /git/test.git/info/refs?service=git-upload-pack HTTP/1.1

to probe for smart-http ?  So this would be ok ?

I'm not sure this is ideal, I don't really know enough about the protocol to 
know 
if I'll see git-receive-pack elsewhere. Possibly if someone includes it in the 
name of a repo it'll blow up in my face.
I can always change it to match only on QUERY_STRING and put the LocationMatch 
back in if that happens.

If that's all that's required, I'm fine with an easy change to httpd.conf

Thanks for the help Jeff.




--
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: git no longer prompting for password

2012-08-26 Thread Jeff King
On Sun, Aug 26, 2012 at 10:57:59AM +0100, Iain Paton wrote:

  The odd URL is because we are probing to see if the server even supports
  smart-http. But note that it does not match your regex above, which
  requires /git-receive-pack. It looks like that is pulled straight from
  the git-http-backend manpage. I think the change in v1.7.8 broke people
  using that configuration.
 
 Yes, it was lifted straight out of the manpage, albeit a couple of years 
 ago now and there have been additions to the manpage since then. 
 I did check, and the basic config is identical in the current manpage.
 
 I can't be the only one using a config that's based on the example in 
 the manpage surely ?  So I'm surprised this hasn't come up previously.

Yeah, I'm surprised it took this long to come up, too. Perhaps most
people just do anonymous http, and then rely on ssh for pushing to
achieve the same effect. Or maybe my analysis of the problem is wrong.
:)

I'm preparing some patches to the test suite that will demonstrate the
problem (we test dumb-http auth, but we don't do any smart-http auth at
all in the test suite), and then a fix on top to let us prompt for the
password in this instance. I think we should also update the
documentation, but the existing advice has been given long enough that
people are going to use it for some time, and I consider your issue to
be a regression in v1.7.8 that should be fixed.

 So after some head scratching trying to work out how to do the equivalent of 
 LocationMatch but on the query string I came up with the following:
 
 ScriptAlias /git/ /usr/libexec/git-core/git-http-backend/
 
 Directory /usr/libexec/git-core
 Require ip 10.44.0.0/16
 If %{THE_REQUEST} =~ /git-receive-pack/
 AuthType Basic
 AuthUserFile /data/git/htpasswd
 AuthGroupfile /data/git/groups
 AuthName Git Access
 
 Require group committers
 /If
 /Directory
 
 and I've removed the LocationMatch section completely.

Yeah, I think that will work. It feels a little weird and hacky. E.g.,
what if you had a repo named git-receive-pack? Unlikely, of course, but
I'd want the config we advertise in the manpage to be as robust as
possible.

I don't know enough about Apache to know off-hand if there is a cleaner
way. I'll investigate a bit more before doing my documentation patch.

 So for accesses to git-http-backend I require auth if anything in the request 
 includes git-receive-pack and that causes a prompt for the username/password 
 as required, while at the same time it still allows anonymous pull.
 
 It appears that the clone operation uses
 
 GET /git/test.git/info/refs?service=git-upload-pack HTTP/1.1
 
 to probe for smart-http ?  So this would be ok ?

Right. Anything invoking receive-pack is always a push.

 I'm not sure this is ideal, I don't really know enough about the protocol to 
 know 
 if I'll see git-receive-pack elsewhere. Possibly if someone includes it in 
 the 
 name of a repo it'll blow up in my face.

Yep, exactly. That should be the only place, though, I think (branch
names, for example, are never part of the URL).

 I can always change it to match only on QUERY_STRING and put the 
 LocationMatch 
 back in if that happens.

I think that would be cleaner. It would be even nicer if you could
really just match service= as a query parameter, but I don't know that
apache parses that at all. I also don't know if Apache does any
canonicalization of the QUERY_STRING. When matching, you'd want to make
sure there is no way of a client sneaking in a parameter that git would
understand to mean a push, but that your pattern would not notice (so,
e.g., just matching git-receive-pack$ would not be sufficient, as I
could request ?service=git-receive-packfooled_you=true. I don't
recall whether git rejects nonsense like that itself.

 If that's all that's required, I'm fine with an easy change to httpd.conf
 
 Thanks for the help Jeff.

No problem. I'll probably be a day or two on the patches, as the http
tests are in need of some refactoring before adding more tests. But in
the meantime, I think your config change is a sane work-around.

-Peff
--
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: git no longer prompting for password

2012-08-26 Thread Iain Paton
On 26/08/12 11:13, Jeff King wrote:

 Yeah, I'm surprised it took this long to come up, too. Perhaps most
 people just do anonymous http, and then rely on ssh for pushing to
 achieve the same effect. Or maybe my analysis of the problem is wrong.
 :)

I'd be using ssh to push too, but the simple fact is that the http way 
works through a proxy and so essentially works from anywhere. The same 
isn't true for ssh or git protocols. Well that's my reason anyway :)

 Yeah, I think that will work. It feels a little weird and hacky. E.g.,

Yeah, it does. I couldn't find a simple way though, most stuff like 
LocationMatch specifically excludes the query string which makes it 
rather more difficult.

 I don't know enough about Apache to know off-hand if there is a cleaner
 way. I'll investigate a bit more before doing my documentation patch.

I'm not an apache expert either. What I could find was using mod_rewrite to 
set an env var based on something in the query string, but not actually do 
any rewrite. Then looking at how to check the env var and do something based 
on that got me the example of simply using If with an expression to match 
directly on the query string.

 I think that would be cleaner. It would be even nicer if you could
 really just match service= as a query parameter, but I don't know that
 apache parses that at all. I also don't know if Apache does any
 canonicalization of the QUERY_STRING. When matching, you'd want to make

From what I can tell apache really doesn't care much about the query string 
at all, it seems to just pass it through unless you start messing with it 
using mod_rewrite, but even then you're still regex based. I couldn't find 
anything that parsed out individual parameters. Of course I could just be 
looking in all the wrong places :) 

 sure there is no way of a client sneaking in a parameter that git would
 understand to mean a push, but that your pattern would not notice (so,
 e.g., just matching git-receive-pack$ would not be sufficient, as I

yep, and matching on THE_REQUEST gets you the whole string, including the 
HTTP/1.1 on the end. I tried putting the $ on the end of the regex and it 
didn't work. 
It should be possible to combine the original regex from the LocationMatch 
example and something like /[?]service=git-receive-pack/ though, which 
should make it somewhat safer.

 No problem. I'll probably be a day or two on the patches, as the http
 tests are in need of some refactoring before adding more tests. But in
 the meantime, I think your config change is a sane work-around.

Works-For-Me is all I need right now :)  I'll be interested if you come 
up with something better though.

Iain

--
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: git no longer prompting for password

2012-08-25 Thread Jeff King
On Sat, Aug 25, 2012 at 03:56:01PM +0100, Iain Paton wrote:

  It's like the initial http requests do not get a 401, and the push
  proceeds, and then some later request causes a 401 when we do not expect
  it. Which is doubly odd, since we should also be able to handle that
  case (the first 401 we get should cause us to ask for a password).
 
 Yes, I deliberately have it set for anonymous pull and authenticated push. 
 So the initial contact with the server doesn't ask for auth.

OK, I see what's going on. It looks like it is configured to do so by
rejecting the POST request. So this first request works:

  GET /git/test.git/info/refs?service=git-receive-pack HTTP/1.1
 User-Agent: git/1.7.8
 Host: 10.44.16.74
 Accept: */*
 Pragma: no-cache
 
  HTTP/1.1 200 OK

which is the first step of the conversation, in which the client gets
the set of refs from the remote. Then it tries to POST the pack:

  POST /git/test.git/git-receive-pack HTTP/1.1
 User-Agent: git/1.7.8
 Host: 10.44.16.74
 Accept-Encoding: deflate, gzip
 Content-Type: application/x-git-receive-pack-request
 Accept: application/x-git-receive-pack-result
 Content-Length: 412
 
 * upload completely sent off: 412 out of 412 bytes
  HTTP/1.1 401 Unauthorized

And we get blocked on that request. I didn't quote it above, but note
how the client actually generates and sends the full pack before being
told no, you can't do this.

So that explains the output you see; we really are generating and
sending the pack, and only then getting a 401. And it also explains why
git does not prompt and retry; we follow a different code path for POSTs
that does not trigger the retry code.

This is not optimal, as we send the pack data only to find out that we
are not authenticated. There is code to avoid sending the _whole_ pack
(it's the probe_rpc code in remote-curl.c), so I think you'd just be
wasting 64K, which is not too bad. So we could teach git to retry if the
POST fails, and I think it would work OK.

But I don't think there is any reason not to block the push request
right from the first receive-pack request we see, which catches the
issue even earlier, and with less overhead (and of course works with
existing git clients :) ).

 apache config has the following:
 [...]
 LocationMatch ^/git/.*/git-receive-pack$
 AuthType Basic
 AuthUserFile /data/git/htpasswd
 AuthGroupfile /data/git/groups 
 AuthName Git Access
 
 Require group committers
 /LocationMatch
 
 nothing untoward there I think and google turns up lots of examples where 
 people are doing essentially the same thing.

I think your regex is the culprit. The first request comes in with:

  GET /git/test.git/info/refs?service=git-receive-pack HTTP/1.1

The odd URL is because we are probing to see if the server even supports
smart-http. But note that it does not match your regex above, which
requires /git-receive-pack. It looks like that is pulled straight from
the git-http-backend manpage. I think the change in v1.7.8 broke people
using that configuration.

I tend to think the right thing is to fix the configuration (both on
your system and in the documentation), but we should probably also fix
git to handle this situation more gracefully, since it used to work and
has been advertised in the documentation for a long time.

-Peff
--
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


git no longer prompting for password

2012-08-24 Thread Iain Paton
Hi List,

A recent update to git 1.7.12 from 1.7.3.5 seems to have changed something - 
trying to push to a smart http backend no longer prompts for a password and 
hence fails the server auth.

The server is currently running git 1.7.9 behind apache 2.4.3 with an almost 
verbatim copy of the apache config from the git-http-backend manpage.

Backtracking through the versions I've skipped and this doesn't seem to be a 
new problem, client side up to 1.7.7.7 works, 1.7.8 onwards don't. Server side 
version doesn't seem to make a difference.

user@fubar01:~/test# git --version
git version 1.7.7.7
user@fubar01:~/test# git push http://ipaton@10.0.0.1/git/test.git master
Password: 

type the password in and the push is successful

user@fubar01:~/test# git --version
git version 1.7.8
user@fubar01:~/test# git push http://ipaton@10.0.0.1/git/test.git master 
--verbose
Pushing to http://ipaton@10.0.0.1/git/test.git
Counting objects: 6, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (3/3), done.
Writing objects: 100% (5/5), 491 bytes, done.
Total 5 (delta 0), reused 0 (delta 0)
error: RPC failed; result=22, HTTP code = 401
fatal: The remote end hung up unexpectedly
fatal: The remote end hung up unexpectedly

Watching the connection with wireshark shows that it does appear to try to 
authenticate with the correct username, but without a password. Not surprising 
since it doesn't ask for one..

googling for git and password just seems to give results where people want it 
to stop asking for a password, which is the oppsite of what I want!  
Looking at changelogs for 1.7.8 and I'm not really seeing anything that says I 
need to do something different.

Any help or pointers appreciated.

Thanks,
Iain

--
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: git no longer prompting for password

2012-08-24 Thread Jeff King
On Fri, Aug 24, 2012 at 09:19:28PM +0100, Iain Paton wrote:

 A recent update to git 1.7.12 from 1.7.3.5 seems to have changed
 something - trying to push to a smart http backend no longer prompts
 for a password and hence fails the server auth.
 [...]
 Backtracking through the versions I've skipped and this doesn't seem
 to be a new problem, client side up to 1.7.7.7 works, 1.7.8 onwards
 don't. Server side version doesn't seem to make a difference.

There was some work in v1.7.8 to avoid prompting for a password when it
is not necessary; I suspect this is a fallout of that.

You could try bisecting the bug. My guess is that you will end up at
commit 986bbc0 (http: don't always prompt for password, 2011-11-04).

 user@fubar01:~/test# git --version
 git version 1.7.7.7
 user@fubar01:~/test# git push http://ipaton@10.0.0.1/git/test.git master
 Password: 

As per the discussion in 986bbc0, this is actually prompting you before
git makes any request. Whereas here:

 user@fubar01:~/test# git --version
 git version 1.7.8
 user@fubar01:~/test# git push http://ipaton@10.0.0.1/git/test.git master 
 --verbose

We should get an HTTP 401 from the server, then prompt, then retry.
What's weird is that it sort of works:

 Pushing to http://ipaton@10.0.0.1/git/test.git
 Counting objects: 6, done.
 Delta compression using up to 8 threads.
 Compressing objects: 100% (3/3), done.
 Writing objects: 100% (5/5), 491 bytes, done.
 Total 5 (delta 0), reused 0 (delta 0)
 error: RPC failed; result=22, HTTP code = 401
 fatal: The remote end hung up unexpectedly
 fatal: The remote end hung up unexpectedly

It's like the initial http requests do not get a 401, and the push
proceeds, and then some later request causes a 401 when we do not expect
it. Which is doubly odd, since we should also be able to handle that
case (the first 401 we get should cause us to ask for a password).

Can you show us the result of running with GIT_CURL_VERBOSE=1? I'd
really like to see which requests are being made with and without
authentication.

 Looking at changelogs for 1.7.8 and I'm not really seeing anything
 that says I need to do something different.

No, you shouldn't need to do anything different. I'd suspect the
weirdness you are seeing is from a credential helper trying to supply a
blank password, except that you would have to have configured one
manually for it to run (I assume you are not on a shared machine where
somebody might have tweaked /etc/gitconfig or anything like that).

-Peff
--
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