git-daemon: needs /root/.config/git/config?

git-daemon: needs /root/.config/git/config?

[Ian Kumlien]

Hi again, 

Due to the earlier problem I upgraded git on all machines 
and eneded up with a ubunut machine running in to problems.

I started getting errors like:
"fatal: protocol error: bad line length character: fata"

Which after some head scratching caused me to tell xinetd to directly
launch git-daemon, eventually it worked fine, but i did get this error
message:

Jun  4 16:12:05 xyz git-daemon[10246]: unable to access
'/root/.config/git/config': Permission denied

It's not the first time i've seen it but i've been able to ignore it
before. This is running as a local user (as in not root) and this user
shouldn't have access to /root. But i eventually had to do chown o+x
/root to workaround this error.

Now, this must be wrong somehow? Or does --user work in inetd mode now?

So... comments, ideas?

Re: git-daemon: needs /root/.config/git/config?

[Jeff King]

On Tue, Jun 04, 2013 at 04:13:14PM +0200, Ian Kumlien wrote:

> Due to the earlier problem I upgraded git on all machines 
> and eneded up with a ubunut machine running in to problems.
> 
> I started getting errors like:
> "fatal: protocol error: bad line length character: fata"
> 
> Which after some head scratching caused me to tell xinetd to directly
> launch git-daemon, eventually it worked fine, but i did get this error
> message:

Looks like your stderr was being redirected to your stdout; this
particular error aside, that is likely to cause weird protocol problems
for any error that git outputs.

> Jun  4 16:12:05 xyz git-daemon[10246]: unable to access
> '/root/.config/git/config': Permission denied
> 
> It's not the first time i've seen it but i've been able to ignore it
> before. This is running as a local user (as in not root) and this user
> shouldn't have access to /root. But i eventually had to do chown o+x
> /root to workaround this error.

The problem is that you have presumably dropped privileges in the daemon
instance, but your $HOME environment variable still points to /root. Git
cannot read all of its config files (nor even find out if they exist),
so it bails rather than continue.

Older versions of git silently ignored errors reading config files, but
it was tightened in v1.8.1.1, as there can be quite serious implications
to failing to read expected config (e.g., imagine transfer.fsckobjects,
or receive.deny* is ignored).

However, since changing user id and leaving $HOME is so common, there is
a patch under consideration to loosen the check only for the case of
EACCES on files in $HOME. That commit is 4698c8f (config: allow
inaccessible configuration under $HOME, 2013-04-12); it's not yet in any
released version of git, though.

In the meantime, the suggested workaround is to set $HOME for the
git-daemon user, rather than loosening /root.

-Peff

Re: git-daemon: needs /root/.config/git/config?

[Johannes Sixt]

Am 04.06.2013 18:08, schrieb Jeff King:
> Older versions of git silently ignored errors reading config files, but
> it was tightened in v1.8.1.1, as there can be quite serious implications
> to failing to read expected config (e.g., imagine transfer.fsckobjects,
> or receive.deny* is ignored).
> 
> However, since changing user id and leaving $HOME is so common, there is
> a patch under consideration to loosen the check only for the case of
> EACCES on files in $HOME. That commit is 4698c8f (config: allow
> inaccessible configuration under $HOME, 2013-04-12); it's not yet in any
> released version of git, though.
> 
> In the meantime, the suggested workaround is to set $HOME for the
> git-daemon user, rather than loosening /root.

I've a PHP script in ~/public_html that runs git. Without the mentioned
patch, the script bails out due to this error. This time it's Apache
that gets me into trouble because at the time the PHP script and git
run, $HOME is still /root, but the user identity is not root anymore.
The patch is direly needed; without it, I need to use 'env
HOME=/home/j6t /usr/local/bin/git' in my script.

-- Hannes

Re: git-daemon: needs /root/.config/git/config?

[Jonathan Nieder]

Johannes Sixt wrote:
> Am 04.06.2013 18:08, schrieb Jeff King:

>> However, since changing user id and leaving $HOME is so common, there is
>> a patch under consideration to loosen the check only for the case of
>> EACCES on files in $HOME. That commit is 4698c8f (config: allow
>> inaccessible configuration under $HOME, 2013-04-12); it's not yet in any
>> released version of git, though.
[...]
> I've a PHP script in ~/public_html that runs git. Without the mentioned
> patch, the script bails out due to this error. This time it's Apache
> that gets me into trouble because at the time the PHP script and git
> run, $HOME is still /root, but the user identity is not root anymore.
> The patch is direly needed; without it, I need to use 'env
> HOME=/home/j6t /usr/local/bin/git' in my script.

I could be remembering wrong, but I thought it was not so much "under
consideration" as "accepted for 1.8.4".  I haven't heard any
compelling reasons not to apply it.

Would it would make sense against earlier releases as well?

Thanks,
Jonathan

Re: git-daemon: needs /root/.config/git/config?

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> I could be remembering wrong, but I thought it was not so much "under
> consideration" as "accepted for 1.8.4".  I haven't heard any
> compelling reasons not to apply it.
>
> Would it would make sense against earlier releases as well?

True; the patch is queued on a topic that forks from maint-1.8.2
exactly for this reason.

Re: git-daemon: needs /root/.config/git/config?

[Jeff King]

On Tue, Jun 04, 2013 at 12:10:25PM -0700, Jonathan Nieder wrote:

> >> However, since changing user id and leaving $HOME is so common, there is
> >> a patch under consideration to loosen the check only for the case of
> >> EACCES on files in $HOME. That commit is 4698c8f (config: allow
> >> inaccessible configuration under $HOME, 2013-04-12); it's not yet in any
> >> released version of git, though.
> [...]
> > I've a PHP script in ~/public_html that runs git. Without the mentioned
> > patch, the script bails out due to this error. This time it's Apache
> > that gets me into trouble because at the time the PHP script and git
> > run, $HOME is still /root, but the user identity is not root anymore.
> > The patch is direly needed; without it, I need to use 'env
> > HOME=/home/j6t /usr/local/bin/git' in my script.
> 
> I could be remembering wrong, but I thought it was not so much "under
> consideration" as "accepted for 1.8.4".  I haven't heard any
> compelling reasons not to apply it.
> 
> Would it would make sense against earlier releases as well?

Yeah, I think it would. I only said "under consideration" because I saw
that it was in "next" and not elsewhere, and did not hunt down the exact
status in "What's Cooking".

-Peff

Re: git-daemon: needs /root/.config/git/config?

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Jun 04, 2013 at 12:10:25PM -0700, Jonathan Nieder wrote:
>
>> >> However, since changing user id and leaving $HOME is so common, there is
>> >> a patch under consideration to loosen the check only for the case of
>> >> EACCES on files in $HOME. That commit is 4698c8f (config: allow
>> >> inaccessible configuration under $HOME, 2013-04-12); it's not yet in any
>> >> released version of git, though.
>> [...]
>> > I've a PHP script in ~/public_html that runs git. Without the mentioned
>> > patch, the script bails out due to this error. This time it's Apache
>> > that gets me into trouble because at the time the PHP script and git
>> > run, $HOME is still /root, but the user identity is not root anymore.
>> > The patch is direly needed; without it, I need to use 'env
>> > HOME=/home/j6t /usr/local/bin/git' in my script.
>> 
>> I could be remembering wrong, but I thought it was not so much "under
>> consideration" as "accepted for 1.8.4".  I haven't heard any
>> compelling reasons not to apply it.
>> 
>> Would it would make sense against earlier releases as well?
>
> Yeah, I think it would. I only said "under consideration" because I saw
> that it was in "next" and not elsewhere, and did not hunt down the exact
> status in "What's Cooking".

Sorry about that; it is in 'master' but "What's cooking" for that
pushout hasn't been published yet (the description of what I did
yesterday will be rolled into today's issue together with the
description of what I did today at the end of day).

Re: git-daemon: needs /root/.config/git/config?

[Ian Kumlien]

On Tue, Jun 04, 2013 at 12:08:15PM -0400, Jeff King wrote:
> On Tue, Jun 04, 2013 at 04:13:14PM +0200, Ian Kumlien wrote:
> 
> > Due to the earlier problem I upgraded git on all machines 
> > and eneded up with a ubunut machine running in to problems.
> > 
> > I started getting errors like:
> > "fatal: protocol error: bad line length character: fata"
> > 
> > Which after some head scratching caused me to tell xinetd to directly
> > launch git-daemon, eventually it worked fine, but i did get this error
> > message:
> 
> Looks like your stderr was being redirected to your stdout; this
> particular error aside, that is likely to cause weird protocol problems
> for any error that git outputs.

Yeah =)

> > Jun  4 16:12:05 xyz git-daemon[10246]: unable to access
> > '/root/.config/git/config': Permission denied
> > 
> > It's not the first time i've seen it but i've been able to ignore it
> > before. This is running as a local user (as in not root) and this user
> > shouldn't have access to /root. But i eventually had to do chown o+x
> > /root to workaround this error.
> 
> The problem is that you have presumably dropped privileges in the daemon
> instance, but your $HOME environment variable still points to /root. Git
> cannot read all of its config files (nor even find out if they exist),
> so it bails rather than continue.

Yeah, assumed =P

> Older versions of git silently ignored errors reading config files, but
> it was tightened in v1.8.1.1, as there can be quite serious implications
> to failing to read expected config (e.g., imagine transfer.fsckobjects,
> or receive.deny* is ignored).

Yes, i agree, it's suboptimal but I for one would use getpwuid to get
the home directory of the executing user to avoid this - though i don't
know how portable it is (or if there is any other issues)

It's a bit hard to control this with xinetd doing it behind the
scenes... 

> However, since changing user id and leaving $HOME is so common, there is
> a patch under consideration to loosen the check only for the case of
> EACCES on files in $HOME. That commit is 4698c8f (config: allow
> inaccessible configuration under $HOME, 2013-04-12); it's not yet in any
> released version of git, though.

Ah, ok, thanks, I'll have a look - maybe i can actually contribute
something for once =)

> In the meantime, the suggested workaround is to set $HOME for the
> git-daemon user, rather than loosening /root.

Well, I have no idea of how to control HOME in xinetd - access to the
machine is limited and x doesn't give that much access (nothing really
important is actually stored in /root)

For now, this is the workaround we have =P

> -Peff

Re: git-daemon: needs /root/.config/git/config?

[Andreas Krey]

On Wed, 05 Jun 2013 13:19:18 +0000, Ian Kumlien wrote:
...
> Well, I have no idea of how to control HOME in xinetd - access to the
> machine is limited and x doesn't give that much access (nothing really
> important is actually stored in /root)

Make xinetd execute '/usr/bin/env HOME=/home/yourstruly git ...'
instead of 'git ...'.

Andreas

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds <torvalds@*.org>
Date: Fri, 22 Jan 2010 07:29:21 -0800

Re: git-daemon: needs /root/.config/git/config?

[Jeff King]

On Wed, Jun 05, 2013 at 01:19:18PM +0200, Ian Kumlien wrote:

> > Older versions of git silently ignored errors reading config files, but
> > it was tightened in v1.8.1.1, as there can be quite serious implications
> > to failing to read expected config (e.g., imagine transfer.fsckobjects,
> > or receive.deny* is ignored).
> 
> Yes, i agree, it's suboptimal but I for one would use getpwuid to get
> the home directory of the executing user to avoid this - though i don't
> know how portable it is (or if there is any other issues)

We considered having git-daemon's "--user" option do that, but:

  1. It would be a regression for people who are intentionally setting
     HOME to get different config profiles. And it would be a surprise
     to admins, as other user-switching daemons (e.g., inetd) do not
     tweak HOME.

  2. It would not have covered all cases, including yours. xinetd is the
     one doing the user-switching here.

-Peff

Re: git-daemon: needs /root/.config/git/config?

[Bernhard R. Link]

* Ian Kumlien <pomac@vapor.com> [130605 13:31]:
> Yes, i agree, it's suboptimal but I for one would use getpwuid to get
> the home directory of the executing user to avoid this - though i don't
> know how portable it is (or if there is any other issues)

It's not only suboptimal but simply wrong. getpwuid gives at best the
initial home directory, and even there it is only a guess. (If you are
looking for some home directory of a different user it might be a good
guess). But using getpwuid(getuid())->pw_dir if HOME is set is a serious
mistake, as you throw out the good value for some almost but not quite
totally unrelated value.

        Bernhard R. Link

Re: git-daemon: needs /root/.config/git/config?

[Ian Kumlien]

On Sun, Jun 09, 2013 at 02:47:57PM +0200, Bernhard R. Link wrote:
> * Ian Kumlien <pomac@vapor.com> [130605 13:31]:
> > Yes, i agree, it's suboptimal but I for one would use getpwuid to get
> > the home directory of the executing user to avoid this - though i don't
> > know how portable it is (or if there is any other issues)
> 
> It's not only suboptimal but simply wrong. getpwuid gives at best the
> initial home directory, and even there it is only a guess. (If you are
> looking for some home directory of a different user it might be a good
> guess). But using getpwuid(getuid())->pw_dir if HOME is set is a serious
> mistake, as you throw out the good value for some almost but not quite
> totally unrelated value.

Well i never intended for it to replace the environment variable, it was
more intended as a fallback - if there will be a "less strict" mode then
perhaps a fallback would be a more controled way of doing it.

>         Bernhard R. Link