{"thread":{"id":"11986","subject":"[PATCH] daemon: Set up PATH properly on startup.","startedAt":"2008-02-09T11:17:53Z","lastAt":"2008-02-12T09:31:05Z","messageCount":3,"participants":["Mark Wooding","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"68069","messageId":"1202555873-8099-1-git-send-email-mdw@distorted.org.uk","threadId":"11986","inReplyTo":null,"subject":"[PATCH] daemon: Set up PATH properly on startup.","fromName":"Mark Wooding","fromEmail":"mdw@distorted.org.uk","sentAt":"2008-02-09T11:17:53Z","receivedAt":"2008-02-09T11:17:53Z","isPatch":true,"sender":{"key":"mdw@distorted.org.uk","avatar":null},"body":"Since exec_cmd.c changed (511707d42b3b3e57d9623493092590546ffeae80) to\njust use the PATH variable for finding Git binaries, the daemon has been\nbroken for people with picky inetds (such as the OpenBSD one) which\nlaunder the environment on startup.  The result is that the daemon\nmysteriously fails to do anything useful.\n\nOne line fix: call setup_paths() in main before doing anything.\n\nSigned-off-by: Mark Wooding <mdw@distorted.org.uk>\n---\n daemon.c |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\n\tI've not addressed the other problem with git-daemon which this\n\tbug brought to my attention, which is that it doesn't log any\n\tkind of error if it fails to exec.\n\ndiff --git a/daemon.c b/daemon.c\nindex 41a60af..cfd6124 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -1149,6 +1149,7 @@ int main(int argc, char **argv)\n \t\tusage(daemon_usage);\n \t}\n \n+\tsetup_path(NULL);\n \tif (inetd_mode && (group_name || user_name))\n \t\tdie(\"--user and --group are incompatible with --inetd\");\n \n-- \n1.5.4.rc5.5.gab98-dirty\n"},{"id":"68252","messageId":"20080210200027.169BD5B0E7@dx.sixt.local","threadId":"11986","inReplyTo":"1202555873-8099-1-git-send-email-mdw@distorted.org.uk","subject":"Re: [PATCH] daemon: Set up PATH properly on startup.","fromName":"Johannes Sixt","fromEmail":"johannes.sixt@telecom.at","sentAt":"2008-02-10T20:00:26Z","receivedAt":"2008-02-10T20:00:26Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Mark Wooding wrote:\n> Since exec_cmd.c changed (511707d42b3b3e57d9623493092590546ffeae80) to\n> just use the PATH variable for finding Git binaries, the daemon has been\n> broken for people with picky inetds (such as the OpenBSD one) which\n> launder the environment on startup.  The result is that the daemon\n> mysteriously fails to do anything useful.\n[...] \n> diff --git a/daemon.c b/daemon.c\n> index 41a60af..cfd6124 100644\n> --- a/daemon.c\n> +++ b/daemon.c\n> @@ -1149,6 +1149,7 @@ int main(int argc, char **argv)\n>  usage(daemon_usage);\n>  }\n>  \n> +     setup_path(NULL);\n>  if (inetd_mode && (group_name || user_name))\n>  die(\"--user and --group are incompatible with --inetd\");\n>  \n\nThere are 2 reason, *not* to do this:\n\n1. It's not needed. You can use\n\n    /usr/local/bin/git --exec-path=/usr/local/bin daemon --inetd ...\n\nto inject the exec-path.\n\n2. Security. Those inetds launder the environment for a reason. Assume inetd\nsets PATH=/usr/bin:/bin and git-daemon is installed\nas /usr/sbin/git-daemon. With your patch now all hooks run with the path\nset to /usr/sbin:/usr/bin:/bin.\n\n-- Hannes\n"},{"id":"68503","messageId":"slrnfr2pqp.gs1.mdw@metalzone.distorted.org.uk","threadId":"11986","inReplyTo":"20080210200027.169BD5B0E7@dx.sixt.local","subject":"Re: [PATCH] daemon: Set up PATH properly on startup.","fromName":"Mark Wooding","fromEmail":"mdw@distorted.org.uk","sentAt":"2008-02-12T09:31:05Z","receivedAt":"2008-02-12T09:31:05Z","isPatch":true,"sender":{"key":"mdw@distorted.org.uk","avatar":null},"body":"Johannes Sixt <johannes.sixt@telecom.at> wrote:\n\n> There are 2 reason, *not* to do this:\n>\n> 1. It's not needed. You can use\n>\n>     /usr/local/bin/git --exec-path=/usr/local/bin daemon --inetd ...\n>\n> to inject the exec-path.\n\nDon't need the `exec-path': git knows the right one already.  So this is\nentirely equivalent to my suggestion, except that the user has to jump\nthrough this stupid hoop.\n\nBesides, I don't want to run `git' from inetd -- it makes distinguishing\nthings in /etc/hosts.deny harder.  Unless I write wrapper scripts or make\nsymlinks for everything, I suppose, but doesn't it seem mad to invent\nwrapper scripts to distinguish between things which are already distinct\nbut glommed together for some strange reason.\n\n> 2. Security. Those inetds launder the environment for a reason. Assume inetd\n> sets PATH=/usr/bin:/bin and git-daemon is installed\n> as /usr/sbin/git-daemon. With your patch now all hooks run with the path\n> set to /usr/sbin:/usr/bin:/bin.\n\nYes, of course.  Silly me.  It's much better that the service fail to\nwork at all than that it have something strange like /usr/local/bin on\nits PATH.\n\nBesides, the only things git-daemon will actually run are\ngit-upload-pack, upload-archive and receive-pack. \n\n  * git-upload-pack in turn runs git-pack-objects, which is a builtin\n    and therefore runs setup_path in main.  Anything that does will\n    therefore certainly have the same evil on its PATH as would have\n    been inserted by my patch -- though I don't think it actually execs\n    anything else.\n\n  * git-upload-archive is another builtin, so the same applies; again, I\n    don't think it actually execs anything else.\n\n  * git-receive-pack is not something you enable if you care about\n    security anyway.\n\nBesides, if there /are/ scripts and so on run by git-daemon, then\nthey'll be hook scripts, and will also fail if the Git tools aren't on\nthe PATH.\n\nYour argument, if I might stoop to caricature, seems to be `No, we\nmustn't have git-daemon set up the path itself -- it might actually\n/work/.'\n\n-- [mdw]\n"}]}