Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-12 Thread Junio C Hamano
Jeff King writes: > OK. I'll hold off for now while we stew on it. Jonathan's patch looks OK > to me, but it has the same issue. But I think every path has to be one > of: > > 1. We annoy sysadmins who need to take an extra step to handle the > HOME situation with --user (the current behav

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-12 Thread Jeff King
On Fri, Apr 12, 2013 at 12:51:19PM -0700, Junio C Hamano wrote: > >> If the access() failed due to ENOENT, the caller will get a negative > >> return from this function and will treat it as "ok, it does not > >> exist", with the original or the updated code. This new case is > >> treated the same

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-12 Thread Junio C Hamano
Jeff King writes: >> If the access() failed due to ENOENT, the caller will get a negative >> return from this function and will treat it as "ok, it does not >> exist", with the original or the updated code. This new case is >> treated the same way by the existing callers, i.e. pretending as if >

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-12 Thread Jeff King
On Fri, Apr 12, 2013 at 11:23:46AM -0700, Junio C Hamano wrote: > Jeff King writes: > > > So here's what I came up with. I tried to make the exception as tight as > > possible by checking that $HOME was actually the problem, as that is the > > common problem (you switch users, but HOME is pointi

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-12 Thread Junio C Hamano
Jeff King writes: > So here's what I came up with. I tried to make the exception as tight as > possible by checking that $HOME was actually the problem, as that is the > common problem (you switch users, but HOME is pointing to the old user). > ... > diff --git a/daemon.c b/daemon.c > index 131b0

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-12 Thread Junio C Hamano
Jeff King writes: > On Fri, Apr 12, 2013 at 09:08:31AM -0700, Junio C Hamano wrote: > >> OK, then... > >> -- >8 -- >> Subject: [PATCH] doc: clarify that "git daemon --user=" option does >> not export HOME=~user > > I'd add this motiviation to the body of the commit message: > > The fact that w

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-12 Thread Jeff King
On Fri, Apr 12, 2013 at 12:16:00PM -0400, Jeff King wrote: > One option we have not explored is an environment variable > to loosen git's requirement. I'm thinking something like > GIT_INACCESSIBLE_HOMEDIR_OK, which could be set by default > when git-daemon uses --user. > > That would leave all e

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-12 Thread Mike Galbraith
On Fri, 2013-04-12 at 09:08 -0700, Junio C Hamano wrote: > Jeff King writes: > > >> How about "and make sure any Git configuration files", since there > >> might not be any Git configuration files. > > > > Yeah, that is better. Thanks. > > OK, then... > > -- >8 -- > Subject: [PATCH] doc: clari

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-12 Thread Jeff King
On Fri, Apr 12, 2013 at 09:08:31AM -0700, Junio C Hamano wrote: > OK, then... > -- >8 -- > Subject: [PATCH] doc: clarify that "git daemon --user=" option does not > export HOME=~user I'd add this motiviation to the body of the commit message: The fact that we don't set $HOME may confuse admi

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-12 Thread Junio C Hamano
Jeff King writes: >> How about "and make sure any Git configuration files", since there >> might not be any Git configuration files. > > Yeah, that is better. Thanks. OK, then... -- >8 -- Subject: [PATCH] doc: clarify that "git daemon --user=" option does not export HOME=~user Signed-off-by:

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-12 Thread Jeff King
On Fri, Apr 12, 2013 at 07:26:36AM -0400, W. Trevor King wrote: > On Fri, Apr 12, 2013 at 01:05:50AM -0400, Jeff King wrote: > > Like many programs that switch user id, the daemon does not reset > > environment variables such a `$HOME` when it runs git programs like > > `upload-pack` and `re

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-12 Thread Evan Priestley
Jonathan Nieder wrote: > I'm not sure whether to keep 96b9e0e (config: treat user and xdg > config permission problem as errors) in the long run, BTW. > > Insights welcome. For what it's worth, here's an anecdote about this: I work on some open source software which includes a web-based reposi

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-12 Thread W. Trevor King
On Fri, Apr 12, 2013 at 01:05:50AM -0400, Jeff King wrote: > Like many programs that switch user id, the daemon does not reset > environment variables such a `$HOME` when it runs git programs like > `upload-pack` and `receive-pack`. When using this option, you may also > want to set and exp

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-11 Thread Mike Galbraith
On Fri, 2013-04-12 at 01:05 -0400, Jeff King wrote: > On Thu, Apr 11, 2013 at 09:46:35PM -0700, Junio C Hamano wrote: > > > >> --user:: > > >> ... current description ... > > >> + > > >> (Like|Unlike) many programs that let you run programs as > > >> specified user, the daemon do

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 09:46:35PM -0700, Junio C Hamano wrote: > >> --user:: > >>... current description ... > >> + > >> (Like|Unlike) many programs that let you run programs as > >> specified user, the daemon does not reset environment variables > >> such as $HOME when it

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-11 Thread Junio C Hamano
Jeff King writes: > On Thu, Apr 11, 2013 at 09:11:20PM -0700, Junio C Hamano wrote: > >> The --user option to git-daemon would be a good place to do that, I >> think. Depending on what other "setuid to less privileged before >> running" programs do (I do not know offhand), we can say something >

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 09:11:20PM -0700, Junio C Hamano wrote: > The --user option to git-daemon would be a good place to do that, I > think. Depending on what other "setuid to less privileged before > running" programs do (I do not know offhand), we can say something > like this perhaps? That'

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-11 Thread Junio C Hamano
"W. Trevor King" writes: > On Thu, Apr 11, 2013 at 06:23:01PM -0400, Jeff King wrote: > ... >> I think the point is that it could add >> >> ...and I was looking in /root, because that is where your HOME points. >> Shouldn't you be able to read your own HOME directory? >> >> which should mak

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-11 Thread W. Trevor King
On Thu, Apr 11, 2013 at 06:23:01PM -0400, Jeff King wrote: > On Thu, Apr 11, 2013 at 03:20:46PM -0700, Junio C Hamano wrote: > > "W. Trevor King" writes: > > > The difficulty was figuring out why the daemon was dying in > > > the first place (which involved bisection for me as well). Maybe > > >

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 03:20:46PM -0700, Junio C Hamano wrote: > "W. Trevor King" writes: > > > As one of the less experienced folks tripped up by this issue, I think > > that setting HOME explicitly before invoking the daemon is simple > > enough (which is why I just fixed my invocation and di

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-11 Thread Junio C Hamano
"W. Trevor King" writes: > As one of the less experienced folks tripped up by this issue, I think > that setting HOME explicitly before invoking the daemon is simple > enough (which is why I just fixed my invocation and didn't post to the > list). Sounds like we need a documentation update somew

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-11 Thread W. Trevor King
On Thu, Apr 11, 2013 at 12:54:34PM -0700, Junio C Hamano wrote: > I also have a suspicion that your patch makes things worse for > people who are more adept at these issues around running daemons > than the people who introduced this problem in the first place (eh, > that's "us")… > > A trade off

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 12:54:34PM -0700, Junio C Hamano wrote: > > I could go either way. I think 96b9e0e is the right thing to do > > conceptually, but I kind of doubt it was affecting all that many people. > > And though it's _possible_ for it to be a security problem, I find it > > much more l

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-11 Thread Junio C Hamano
Jeff King writes: > On Thu, Apr 11, 2013 at 11:11:03AM -0700, Jonathan Nieder wrote: > >> > -- >8 -- >> > Subject: [PATCH] daemon: set HOME when we switch to --user >> >> Thanks for taking care of it. For what it's worth, >> >> Acked-by: Jonathan Nieder >> >> I'm not sure whether to keep 96b

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-11 Thread Jonathan Nieder
Jeff King wrote: > I could go either way. I think 96b9e0e is the right thing to do > conceptually, but I kind of doubt it was affecting all that many people. > And though it's _possible_ for it to be a security problem, I find it > much more likely that the site admin tries to set some config, get

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 11:11:03AM -0700, Jonathan Nieder wrote: > > -- >8 -- > > Subject: [PATCH] daemon: set HOME when we switch to --user > > Thanks for taking care of it. For what it's worth, > > Acked-by: Jonathan Nieder > > I'm not sure whether to keep 96b9e0e (config: treat user and xd

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-11 Thread Jonathan Nieder
Jeff King wrote: > Here it is with a commit message. > > -- >8 -- > Subject: [PATCH] daemon: set HOME when we switch to --user Thanks for taking care of it. For what it's worth, Acked-by: Jonathan Nieder I'm not sure whether to keep 96b9e0e (config: treat user and xdg config permission proble

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 08:35:46AM -0700, Junio C Hamano wrote: > > Yeah, I think the bug here is that git-daemon should be setting $HOME > > when it switches privileges with --user. Does this patch fix it for you? > [...] > Yeah, that sounds like the obvious fix to me. Here it is with a commit m

Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon

2013-04-11 Thread Junio C Hamano
Jeff King writes: > On Thu, Apr 11, 2013 at 05:39:43AM +0200, Mike Galbraith wrote: > >> > ALLOWED_ENV="PATH HOME" >> > HOME=/ >> >> I can work around it by changing the init script to use su - git -c "bla >> bla" to launch the thing, instead of using --user=git --group=daemon, >> but that's