Re: git-daemon regression: 650c449250d7 common-main: call git_extract_argv0_path()
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()
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
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
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
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
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
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
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