Re: git-daemon regression: 650c449250d7 common-main: call git_extract_argv0_path()

2016-11-26 Thread Mike Galbraith
On Sat, 2016-11-26 at 12:09 -0500, Jeff King wrote:
> On Sat, Nov 26, 2016 at 03:03:48PM +0100, Mike Galbraith wrote:
> 
> > git-daemon went broke on me post v2.9.3 due to binaries being installed
> > in /usr/lib/git, which is not in PATH.  Reverting 650c449250d7 fixes it
> > up, as does ln -s /usr/lib/git/git-daemon /usr/bin/git-daemon 'course,
> > but thought I should report it, since it used to work without that.
> 
> Generally /usr/lib/git _should_ be in your PATH, as it is added by the
> git wrapper when you run "git daemon".
> 
> The only behavior difference caused by 650c449250d7 is that we replace
> argv[0] with the output of git_extract_argv0_path(argv[0]), which will
> give the basename, not a full path. So presumably you are running:
> 
>   /usr/lib/git/git-daemon
> 
> directly. I'm not sure that's even supposed to work these days, and it
> was not just a happy accident that it did.

Ah.  I'm using suse's rpm glue to package my modified source, and its
startup script still calls it directly, so wants some modernization.

> On the other hand, I am sympathetic that something used to work and now
> doesn't. It probably wouldn't be that hard to work around it.
> 
> The reason for the behavior change is that one of the cmd_main()
> functions was relying on the basename side-effect of the
> extract_argv0_path function, so 650c449250d7 just feeds the munged
> argv[0] to all of the programs. The cleanest fix would probably be
> something like:

That did fix it up, thanks.  I'll try twiddling the script instead.

> diff --git a/common-main.c b/common-main.c
> index 44a29e8b1..c654f9555 100644
> --- a/common-main.c
> +++ b/common-main.c
> @@ -33,7 +33,7 @@ int main(int argc, const char **argv)
>  
>  >> git_setup_gettext();
>  
> ->> argv[0] = git_extract_argv0_path(argv[0]);
> +>> git_extract_argv0_path(argv[0]);
>  
>  >> restore_sigpipe_to_default();
>  
> diff --git a/git.c b/git.c
> index bd66a2e0a..05986680c 100644
> --- a/git.c
> +++ b/git.c
> @@ -730,6 +730,11 @@ int cmd_main(int argc, const char **argv)
>  >> cmd = argv[0];
>  >> if (!cmd)
>  >>   > cmd = "git-help";
> +>> else {
> +>>   > const char *base = find_last_dir_sep(cmd);
> +>>   > if (base)
> +>>   >   > cmd = base + 1;
> +>> }
>  
>  >> trace_command_performance(argv);
>  >> trace_stdin();
> 
> -Peff


git-daemon regression: 650c449250d7 common-main: call git_extract_argv0_path()

2016-11-26 Thread Mike Galbraith
Greetings,

git-daemon went broke on me post v2.9.3 due to binaries being installed
in /usr/lib/git, which is not in PATH.  Reverting 650c449250d7 fixes it
up, as does ln -s /usr/lib/git/git-daemon /usr/bin/git-daemon 'course,
but thought I should report it, since it used to work without that.

Process 18804 attached
restart_syscall(<... resuming interrupted call ...>) = 1
accept(4, {sa_family=AF_INET6, sin6_port=htons(44400), inet_pton(AF_INET6, 
"::1", _addr), sin6_flowinfo=0, sin6_scope_id=0}, [28]) = 5
dup(5)  = 6
pipe([7, 8])= 0
clone(Process 18830 attached
child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, 
child_tidptr=0x7f0e6061c9d0) = 18830
[pid 18830] set_robust_list(0x7f0e6061c9e0, 24 
[pid 18804] close(8 
[pid 18830] <... set_robust_list resumed> ) = 0
[pid 18804] <... close resumed> )   = 0
[pid 18804] read(7,  
[pid 18830] close(7)= 0
[pid 18830] fcntl(8, F_GETFD)   = 0
[pid 18830] fcntl(8, F_SETFD, FD_CLOEXEC) = 0
[pid 18830] dup2(5, 0)  = 0
[pid 18830] close(5)= 0
[pid 18830] dup2(6, 1)  = 1
[pid 18830] close(6)= 0
[pid 18830] execve("/usr/local/sbin/git-daemon", ["git-daemon", "--serve", 
"--syslog", "--detach", "--reuseaddr", "--user=git", "--group=daemon", 
"--pid-file=/var/run/git-daemon.p"..., "--export-all", "--user-path"], 
["CONSOLE=/dev/console", "LC_ALL=POSIX", "REDIRECT=/dev/tty7", "COLUMNS=320", 
"PATH=/usr/local/sbin:/usr/local/"..., "PWD=/", "LINES=90", "SHLVL=1", 
"LC_CTYPE=en_US.UTF-8", "_=/sbin/startproc", "PREVLEVEL=", "RUNLEVEL=5", 
"DAEMON=/usr/lib/git/git-daemon", "REMOTE_ADDR=[::1]", "REMOTE_PORT=44400"]) = 
-1 ENOENT (No such file or directory)
[pid 18830] execve("/usr/local/bin/git-daemon", ["git-daemon", "--serve", 
"--syslog", "--detach", "--reuseaddr", "--user=git", "--group=daemon", 
"--pid-file=/var/run/git-daemon.p"..., "--export-all", "--user-path"], 
["CONSOLE=/dev/console", "LC_ALL=POSIX", "REDIRECT=/dev/tty7", "COLUMNS=320", 
"PATH=/usr/local/sbin:/usr/local/"..., "PWD=/", "LINES=90", "SHLVL=1", 
"LC_CTYPE=en_US.UTF-8", "_=/sbin/startproc", "PREVLEVEL=", "RUNLEVEL=5", 
"DAEMON=/usr/lib/git/git-daemon", "REMOTE_ADDR=[::1]", "REMOTE_PORT=44400"]) = 
-1 ENOENT (No such file or directory)
[pid 18830] execve("/usr/sbin/git-daemon", ["git-daemon", "--serve", 
"--syslog", "--detach", "--reuseaddr", "--user=git", "--group=daemon", 
"--pid-file=/var/run/git-daemon.p"..., "--export-all", "--user-path"], 
["CONSOLE=/dev/console", "LC_ALL=POSIX", "REDIRECT=/dev/tty7", "COLUMNS=320", 
"PATH=/usr/local/sbin:/usr/local/"..., "PWD=/", "LINES=90", "SHLVL=1", 
"LC_CTYPE=en_US.UTF-8", "_=/sbin/startproc", "PREVLEVEL=", "RUNLEVEL=5", 
"DAEMON=/usr/lib/git/git-daemon", "REMOTE_ADDR=[::1]", "REMOTE_PORT=44400"]) = 
-1 ENOENT (No such file or directory)
[pid 18830] execve("/usr/bin/git-daemon", ["git-daemon", "--serve", "--syslog", 
"--detach", "--reuseaddr", "--user=git", "--group=daemon", 
"--pid-file=/var/run/git-daemon.p"..., "--export-all", "--user-path"], 
["CONSOLE=/dev/console", "LC_ALL=POSIX", "REDIRECT=/dev/tty7", "COLUMNS=320", 
"PATH=/usr/local/sbin:/usr/local/"..., "PWD=/", "LINES=90", "SHLVL=1", 
"LC_CTYPE=en_US.UTF-8", "_=/sbin/startproc", "PREVLEVEL=", "RUNLEVEL=5", 
"DAEMON=/usr/lib/git/git-daemon", "REMOTE_ADDR=[::1]", "REMOTE_PORT=44400"]) = 
-1 ENOENT (No such file or directory)
[pid 18830] execve("/sbin/git-daemon", ["git-daemon", "--serve", "--syslog", 
"--detach", "--reuseaddr", "--user=git", "--group=daemon", 
"--pid-file=/var/run/git-daemon.p"..., "--export-all", "--user-path"], 
["CONSOLE=/dev/console", "LC_ALL=POSIX", "REDIRECT=/dev/tty7", "COLUMNS=320", 
"PATH=/usr/local/sbin:/usr/local/"..., "PWD=/", "LINES=90", "SHLVL=1", 
"LC_CTYPE=en_US.UTF-8", "_=/sbin/startproc", "PREVLEVEL=", "RUNLEVEL=5", 
"DAEMON=/usr/lib/git/git-daemon", "REMOTE_ADDR=[::1]", "REMOTE_PORT=44400"]) = 
-1 ENOENT (No such file or directory)
[pid 18830] execve("/bin/git-daemon", ["git-daemon", "--serve", "--syslog", 
"--detach", "--reuseaddr", "--user=git", "--group=daemon", 
"--pid-file=/var/run/git-daemon.p"..., "--export-all", "--user-path"], 
["CONSOLE=/dev/console", "LC_ALL=POSIX", "REDIRECT=/dev/tty7", "COLUMNS=320", 
"PATH=/usr/local/sbin:/usr/local/"..., "PWD=/", "LINES=90", "SHLVL=1", 
"LC_CTYPE=en_US.UTF-8", "_=/sbin/startproc", "PREVLEVEL=", "RUNLEVEL=5", 
"DAEMON=/usr/lib/git/git-daemon", "REMOTE_ADDR=[::1]", "REMOTE_PORT=44400"]) = 
-1 ENOENT (No such file or directory)
[pid 18830] fstat(2, {st_dev=makedev(0, 6), st_ino=1029, st_mode=S_IFCHR|0666, 
st_nlink=1, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=0, 
st_rdev=makedev(1, 3), st_atime=2016/11/26-00:42:47, 
st_mtime=2016/11/26-00:42:47, st_ctime=2016/11/26-00:42:47}) = 0
[pid 18830] ioctl(2, TCGETS, 0x7ffe6dbd09b0) = -1 ENOTTY (Inappropriate ioctl 
for device)
[pid 18830] mmap(NULL, 4096, PROT_READ|PROT_WRITE, 

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 p...@peff.net 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=user option does not 
 export HOME=~user
 
 Signed-off-by: Jeff King p...@peff.net
 Helped-by: W. Trevor King wk...@tremily.us
 Signed-off-by: Junio C Hamano gits...@pobox.com
 ---
  Documentation/git-daemon.txt | 7 +++
  1 file changed, 7 insertions(+)
 
 diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
 index 7e5098a..2ac07ba 100644
 --- a/Documentation/git-daemon.txt
 +++ b/Documentation/git-daemon.txt
 @@ -147,6 +147,13 @@ OPTIONS
  Giving these options is an error when used with `--inetd`; use
  the facility of inet daemon to achieve the same before spawning
  'git daemon' if needed.
 ++
 +Like many programs that switch user id, the daemon does not reset
 +environment variables such as `$HOME` when it runs git programs,
 +e.g. `upload-pack` and `receive-pack`. When using this option, you
 +may also want to set and export `HOME` to point at the home
 +directory of `user` before starting the daemon, and make sure any
 +Git configuration files in that directory are readable by `user`.

The you may want to.. is perhaps a little understated given it will
fail -EGOAWAY if git-daemon is started via init scripts if you don't.
(but otoh, that's enough of a hint to anyone setting the thing up, no
need to write paragraphs of legal-beagle boiler-plate for dinky bug;)

-Mike 

--
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: [PATCH v2] config: allow inaccessible configuration under $HOME

2013-04-12 Thread Mike Galbraith
Tested, original setup works fine.

On Fri, 2013-04-12 at 14:03 -0700, Jonathan Nieder wrote: 
 The changes v1.7.12.1~2^2~4 (config: warn on inaccessible files,
 2012-08-21) and v1.8.1.1~22^2~2 (config: treat user and xdg config
 permission problems as errors, 2012-10-13) were intended to prevent
 important configuration (think [transfer] fsckobjects) from being
 ignored when the configuration is unintentionally unreadable (for
 example with EIO on a flaky filesystem, or with ENOMEM due to a DoS
 attack).  Usually ~/.gitconfig and ~/.config/git are readable by the
 current user, and if they aren't then it would be easy to fix those
 permissions, so the damage from adding this check should have been
 minimal.
 
 Unfortunately the access() check often trips when git is being run as
 a server.  A daemon (such as inetd or git-daemon) starts as root,
 creates a listening socket, and then drops privileges, meaning that
 when git commands are invoked they cannot access $HOME and die with
 
  fatal: unable to access '/root/.config/git/config': Permission denied
 
 Any patch to fix this would have one of three problems:
 
   1. We annoy sysadmins who need to take an extra step to handle HOME
  when dropping privileges (the current behavior, or any other
  proposal that they have to opt into).
 
   2. We annoy sysadmins who want to set HOME when dropping privileges,
  either by making what they want to do impossible, or making them
  set an extra variable or option to accomplish what used to work
  (e.g., a patch to git-daemon to set HOME when --user is passed).
 
   3. We loosen the check, so some cases which might be noteworthy are
  not caught.
 
 This patch is of type (3).
 
 Treat user and xdg configuration that are inaccessible due to
 permissions (EACCES) as though no user configuration was provided at
 all.
 
 An alternative method would be to check if $HOME is readable, but that
 would not help in cases where the user who dropped privileges had a
 globally readable HOME with only .config or .gitconfig being private.
 
 This does not change the behavior when /etc/gitconfig or .git/config
 is unreadable (since those are more serious configuration errors),
 nor when ~/.gitconfig or ~/.config/git is unreadable due to problems
 other than permissions.
 
 Signed-off-by: Jonathan Nieder jrnie...@gmail.com
 Improved-by: Jeff King p...@peff.net
 ---
 Jonathan Nieder wrote:
 
  --- a/wrapper.c
  +++ b/wrapper.c
  @@ -408,11 +408,16 @@ void warn_on_inaccessible(const char *path)
  warning(_(unable to access '%s': %s), path, strerror(errno));
   }
   
  +static int access_error_is_ok(int err, unsigned flag)
  +{
  +   return errno == ENOENT || errno == ENOTDIR ||
 
 Sigh, I can't spell err.  Here's a v2 incorporating that change and
 with commit message incorporating the latest discussion.
 
  builtin/config.c  |  4 ++--
  config.c  | 10 +-
  dir.c |  4 ++--
  git-compat-util.h |  5 +++--
  wrapper.c | 14 ++
  5 files changed, 22 insertions(+), 15 deletions(-)
 
 diff --git a/builtin/config.c b/builtin/config.c
 index 33c9bf9..19ffcaf 100644
 --- a/builtin/config.c
 +++ b/builtin/config.c
 @@ -379,8 +379,8 @@ int cmd_config(int argc, const char **argv, const char 
 *prefix)
*/
   die($HOME not set);
  
 - if (access_or_warn(user_config, R_OK) 
 - xdg_config  !access_or_warn(xdg_config, R_OK))
 + if (access_or_warn(user_config, R_OK, 0) 
 + xdg_config  !access_or_warn(xdg_config, R_OK, 0))
   given_config_file = xdg_config;
   else
   given_config_file = user_config;
 diff --git a/config.c b/config.c
 index aefd80b..830ee14 100644
 --- a/config.c
 +++ b/config.c
 @@ -58,7 +58,7 @@ static int handle_path_include(const char *path, struct 
 config_include_data *inc
   path = buf.buf;
   }
  
 - if (!access_or_die(path, R_OK)) {
 + if (!access_or_die(path, R_OK, 0)) {
   if (++inc-depth  MAX_INCLUDE_DEPTH)
   die(include_depth_advice, MAX_INCLUDE_DEPTH, path,
   cf  cf-name ? cf-name : the command line);
 @@ -954,23 +954,23 @@ int git_config_early(config_fn_t fn, void *data, const 
 char *repo_config)
  
   home_config_paths(user_config, xdg_config, config);
  
 - if (git_config_system()  !access_or_die(git_etc_gitconfig(), R_OK)) {
 + if (git_config_system()  !access_or_die(git_etc_gitconfig(), R_OK, 
 0)) {
   ret += git_config_from_file(fn, git_etc_gitconfig(),
   data);
   found += 1;
   }
  
 - if (xdg_config  !access_or_die(xdg_config, R_OK)) {
 + if (xdg_config  !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK)) {
   ret += git_config_from_file(fn, xdg_config, data);
   found += 1;
   }
  
 - if 

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

2013-04-11 Thread Mike Galbraith
On Thu, 2013-04-11 at 01:42 -0400, Jeff King wrote: 
 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 just a bandaid for the busted environment setup those
  switches were supposed to make happen, no?
 
 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?
 
 diff --git a/daemon.c b/daemon.c
 index 6aeddcb..a4451fd 100644
 --- a/daemon.c
 +++ b/daemon.c
 @@ -1091,6 +1091,7 @@ static void drop_privileges(struct credentials *cred)
   if (cred  (initgroups(cred-pass-pw_name, cred-gid) ||
   setgid (cred-gid) || setuid(cred-pass-pw_uid)))
   die(cannot drop privileges);
 + setenv(HOME, cred-pass-pw_dir, 1);
  }
  
  static struct credentials *prepare_credentials(const char *user_name,
 
 I guess that would technically break anybody who was trying to do
 something clever with HOME (i.e., point it somewhere besides --user's
 HOME where they had put some config files). But the obvious clever
 thing would be to also set the user's passwd homedir to the same place.

I did exactly that yesterday, and it didn't work, which rather surprised
me.  Let me double check that I didn't screw trivial all up...

Heh, if ya don't plunk new binary into the old oddly placed bucket :)
Yeah, that works just fine.

-Mike 


--
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: 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 does not reset environment variables
   such as $HOME when it runs git programs like upload-pack and
   receive-pack. Set and export HOME to point at the home directory
   of the user you specify with this option before you start the
   daemon, and make sure the Git configuration files in that
   directory is readable by that user.
  
   So choosing Like here, I think this makes sense.
  
  I would prefer the simplicity ;-)
  
  Set and export HOME to point at the home directory of the user you
  specify with this option screams that it wants to be rephrased at
  me, though.  It somehow sounds as if this option is a way to set and
  export the environment variable unless re-read carefully X-.
 
 Perhaps:
 
   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 export `HOME` to point at the home directory of
   `user` before starting the daemon, and make sure the Git
   configuration file in that directory are readable by `user`.
 
 I tried to address your concern above (which I agree with), smooth over
 a few clunky wordings, and use user, which is defined in the heading
 of the option:
 
   --user=user, --group=group

I just updated my local rpm to set HOME, so the surprise of no workee
after upgrade here is forever gone.  Looks like suses (at least)
packagers will need to do the same, else canned setup ain't gonna work.

-Mike

--
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] regression: 96b9e0e3 config: treat user and xdg config permission problems as errors busted git-daemon

2013-04-10 Thread Mike Galbraith
On Wed, 2013-04-10 at 09:56 -0400, W. Trevor King wrote: 
 On Wed, Apr 10, 2013 at 07:33:35AM +0200, Mike Galbraith wrote:
  /usr/lib/git/git-daemon --syslog --detach --reuseaddr --user=git 
  --group=daemon --pid-file=/var/run/git-daemon.pid --export-all --user-path 
  --enable=receive-pack
  
  Try to pull as root or normal user results in:
  
  [pid 26786] access(/root/.config/git/config, R_OK) = -1 EACCES 
  (Permission denied)
  [pid 26786] write(2, fatal: unable to access '/root/, 70) = 70
  [pid 26785] ... read resumed fatal: unable to access '/root/, 4096) 
  = 70
  [pid 26786] exit_group(128)
  
  Bisection fingered this commit, though it looks like it's really due to
  not forgetting who it was at birth.  It's not root, so has no business
  rummaging around in /root.  It used to not care, but this commit made
  go away while looking for non-existent config file terminal.
 
 I ran into this too, although I'm running git-daemon via spawn-fcgi.
 In order to convince newer Gits that you know what you're doing, you
 just need to set HOME to somewhere Git can look.  For example:
 
   HOME=/ /usr/lib/git/git-daemon …
 
 should work.  On Gentoo, I added the following to
 /etc/conf.d/spawn-fcgi.fcgiwrap:
 
   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 just a bandaid for the busted environment setup those
switches were supposed to make happen, no?

-Mike

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


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

2013-04-09 Thread Mike Galbraith
Greetings,

I use git-daemon as the keeper of all source (love it).  git is a normal
user, running as git:daemon, with all repositories living in ~git.

git-daemon is started like so:

/usr/lib/git/git-daemon --syslog --detach --reuseaddr --user=git --group=daemon 
--pid-file=/var/run/git-daemon.pid --export-all --user-path 
--enable=receive-pack

Try to pull as root or normal user results in:

[pid 26786] access(/root/.config/git/config, R_OK) = -1 EACCES (Permission 
denied)
[pid 26786] write(2, fatal: unable to access '/root/, 70) = 70
[pid 26785] ... read resumed fatal: unable to access '/root/, 4096) = 70
[pid 26786] exit_group(128)

Bisection fingered this commit, though it looks like it's really due to
not forgetting who it was at birth.  It's not root, so has no business
rummaging around in /root.  It used to not care, but this commit made
go away while looking for non-existent config file terminal.

-Mike

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