{"thread":{"id":"1806","subject":"git-daemon --inetd","startedAt":"2005-09-15T06:04:47Z","lastAt":"2005-10-22T21:05:38Z","messageCount":15,"participants":["H. Peter Anvin","Linus Torvalds","Junio C Hamano","Martin Langhoff","Jon Seymour"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"8584","messageId":"43290EFF.3070604@zytor.com","threadId":"1806","inReplyTo":null,"subject":"git-daemon --inetd","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-09-15T06:04:47Z","receivedAt":"2005-09-15T06:04:47Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Some people have asked why we haven't enabled git-daemon on kernel.org. \n  The reason is that git-daemon --inetd is broken; there is a patch in \nthe current repository which claims to fix it, but there isn't a release \nversion yet.\n\nOn that note, though, it would be very nice if there was a way to run \ngit-daemon in a chroot, or otherwise restrict it to a specific \nsubhierarchy of the filesystem.\n\n\t-hpa\n"},{"id":"8606","messageId":"Pine.LNX.4.58.0509150829090.26803@g5.osdl.org","threadId":"1806","inReplyTo":"43290EFF.3070604@zytor.com","subject":"Re: git-daemon --inetd","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-09-15T16:03:31Z","receivedAt":"2005-09-15T16:03:31Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 14 Sep 2005, H. Peter Anvin wrote:\n> \n> On that note, though, it would be very nice if there was a way to run \n> git-daemon in a chroot, or otherwise restrict it to a specific \n> subhierarchy of the filesystem.\n\nHmm.. That should work fine. You can already just run it that way by just \nwrapping it in \"chroot\", but if you don't want that for some reason, how \nabout a patch like this?\n\nIt compiles. That's just about all I can say about it.\n\n\t\tLinus\n\n---\ndiff --git a/daemon.c b/daemon.c\n--- a/daemon.c\n+++ b/daemon.c\n@@ -1,11 +1,19 @@\n+/* setresuid/setresgid. GNU? Who are you kidding, glibc? */\n+#define _GNU_SOURCE\n+\n #include \"cache.h\"\n #include \"pkt-line.h\"\n #include <signal.h>\n #include <sys/wait.h>\n #include <sys/socket.h>\n #include <sys/time.h>\n+#include <sys/types.h>\n #include <netdb.h>\n #include <netinet/in.h>\n+#include <unistd.h>\n+\n+#include <pwd.h>\n+#include <grp.h>\n \n static const char daemon_usage[] = \"git-daemon [--inetd | --port=n]\";\n \n@@ -326,10 +334,50 @@ static int serve(int port)\n \t}\n }\n \n+/*\n+ * This will drop our effective uid/gid to the real one,\n+ * even if no \"--user=\" or \"--group=\" arguments were used.\n+ *\n+ * So it's not a no-op even without that.\n+ */\n+static void set_user_group(const char *user, const char *group)\n+{\n+\tuid_t uid = getuid();\n+\tgid_t gid = getgid();\n+\tchar *end;\n+\n+\tif (user) {\n+\t\tstruct passwd *pw = getpwnam(user);\n+\t\tif (pw) {\n+\t\t\tuid = pw->pw_uid;\n+\t\t\tgid = pw->pw_gid;\n+\t\t} else {\n+\t\t\tuid = strtol(user, &end, 10);\n+\t\t\tif (*end || end == user)\n+\t\t\t\tdie(\"invalid user name '%s'\", user);\n+\t\t}\n+\t}\n+\tif (group) {\n+\t\tstruct group *grp = getgrnam(group);\n+\t\tif (grp) {\n+\t\t\tgid = grp->gr_gid;\n+\t\t} else {\n+\t\t\tgid = strtol(group, &end, 10);\n+\t\t\tif (*end || end == group)\n+\t\t\t\tdie(\"invalid group name '%s'\", group);\n+\t\t}\n+\t}\n+\tif (!uid || !gid)\n+\t\tdie(\"I refuse to run as root\");\n+\tif (setresgid(gid, gid, gid) || setresuid(uid, uid, uid))\n+\t\tdie(\"I can't run as %d:%d\", uid, gid);\n+}\n+\n int main(int argc, char **argv)\n {\n \tint port = DEFAULT_GIT_PORT;\n \tint inetd_mode = 0;\n+\tconst char *user = NULL, *group = NULL;\n \tint i;\n \n \tfor (i = 1; i < argc; i++) {\n@@ -350,9 +398,32 @@ int main(int argc, char **argv)\n \t\t\tcontinue;\n \t\t}\n \n+\t\tif (!strncmp(arg, \"--chroot=\", 9)) {\n+\t\t\tif (chroot(arg+9) < 0)\n+\t\t\t\tdie(\"unable to chroot to '%s': %s\", arg+9, strerror(errno));\n+\t\t\tif (chdir(\"/\") < 0)\n+\t\t\t\tdie(\"unable to chdir to new root\");\n+\n+\t\t\tuser = user ? user : \"nobody\";\n+\t\t\tgroup = group ? group : \"nobody\";\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (!strncmp(arg, \"--user=\", 7)) {\n+\t\t\tuser = arg+7;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (!strncmp(arg, \"--group=\", 8)) {\n+\t\t\tgroup = arg + 8;\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\tusage(daemon_usage);\n \t}\n \n+\tset_user_group(user, group);\n+\n \tif (inetd_mode) {\n \t\tfclose(stderr); //FIXME: workaround\n \t\treturn execute();\n"},{"id":"8616","messageId":"4329BDD9.4010507@zytor.com","threadId":"1806","inReplyTo":"Pine.LNX.4.58.0509150829090.26803@g5.osdl.org","subject":"Re: git-daemon --inetd","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-09-15T18:30:49Z","receivedAt":"2005-09-15T18:30:49Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Linus Torvalds wrote:\n> \n> Hmm.. That should work fine. You can already just run it that way by just \n> wrapping it in \"chroot\", but if you don't want that for some reason, how \n> about a patch like this?\n> \n> It compiles. That's just about all I can say about it.\n> \n\nWrapping it in chroot() would mean having enough things in the chroot \nenvironment to support starting up programs, which is ugly.  I'll test \nthe patch when I get a chance.\n\n\t-hpa\n"},{"id":"8619","messageId":"Pine.LNX.4.58.0509151142570.26803@g5.osdl.org","threadId":"1806","inReplyTo":"4329BDD9.4010507@zytor.com","subject":"Re: git-daemon --inetd","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-09-15T18:47:16Z","receivedAt":"2005-09-15T18:47:16Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 15 Sep 2005, H. Peter Anvin wrote:\n> \n> Wrapping it in chroot() would mean having enough things in the chroot \n> environment to support starting up programs, which is ugly.  I'll test \n> the patch when I get a chance.\n\nFair enough. This still requires \"git-upload-pack\" and any libraries that\nrequires, of course. You might want to do a statically linked version or\nsomething.\n\nHmm. A quick check seems to say that git-upload-pack only does libz and \nlibc. And the SHA1 stuff, of course, but that you can always get static by \nusing the git built-in mozilla versions.\n\n\t\t\tLinus\n"},{"id":"8623","messageId":"4329C93D.2020701@zytor.com","threadId":"1806","inReplyTo":"Pine.LNX.4.58.0509151142570.26803@g5.osdl.org","subject":"Re: git-daemon --inetd","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-09-15T19:19:25Z","receivedAt":"2005-09-15T19:19:25Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Linus Torvalds wrote:\n> \n> On Thu, 15 Sep 2005, H. Peter Anvin wrote:\n> \n>>Wrapping it in chroot() would mean having enough things in the chroot \n>>environment to support starting up programs, which is ugly.  I'll test \n>>the patch when I get a chance.\n> \n> \n> Fair enough. This still requires \"git-upload-pack\" and any libraries that\n> requires, of course. You might want to do a statically linked version or\n> something.\n> \n> Hmm. A quick check seems to say that git-upload-pack only does libz and \n> libc. And the SHA1 stuff, of course, but that you can always get static by \n> using the git built-in mozilla versions.\n> \n\nCould git-upload-pack be integrated into the git-daemon binary, so that \nthe exec isn't needed?\n\n\t-hpa\n"},{"id":"8628","messageId":"Pine.LNX.4.58.0509151225410.26803@g5.osdl.org","threadId":"1806","inReplyTo":"4329C93D.2020701@zytor.com","subject":"Re: git-daemon --inetd","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-09-15T19:40:54Z","receivedAt":"2005-09-15T19:40:54Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 15 Sep 2005, H. Peter Anvin wrote:\n> \n> Could git-upload-pack be integrated into the git-daemon binary, so that \n> the exec isn't needed?\n\nHmm.. Looking a bit closer, that's not really very helpful. \n\ngit-upload-pack ends up fork/exec'ing git-rev-list too, to generate the \nlist of commits. And then it ends up finishing off with \n\"git-pack-objects\". \n\nYes, we could make it all be one huge thing, and try to have lots of \nfunctional interfaces. But that would be really quite ugly and big changes \n(they'd have to be asynchronous and give incremental results etc etc).\n\nMaybe a post-1.0 effort. For now, I think you need to have a minimal\nexecution environment inside the chroot. Not a lot: git-upload-pack,\ngit-rev-list and git-pack-objects.\n\nNow, it turns out that git-rev-list really is the most complex of them \nall, largely due to --merge-order. If you define NO_OPENSSL, you'll get a \nmuch simpler execution environment.\n\nBtw, I think --merge-order was cool, but its weaker cousin --topo-order is\nwhat is actually _used_. Maybe we should deprecate --merge-order? Right\nnow the only real user is git-archimport, and I think that one too really\nonly wants topo-order too. For example, right now I think git-archimport \nwon't actually work if you build without openssl.\n\nJon? Martin?\n\n\t\tLinus\n"},{"id":"8631","messageId":"7vek7qyuhd.fsf@assigned-by-dhcp.cox.net","threadId":"1806","inReplyTo":"4329C93D.2020701@zytor.com","subject":"Re: git-daemon --inetd","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-09-15T19:57:34Z","receivedAt":"2005-09-15T19:57:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"H. Peter Anvin\" <hpa@zytor.com> writes:\n\n> Could git-upload-pack be integrated into the git-daemon binary, so that \n> the exec isn't needed?\n\nIn the longer term, I'd like to extend git-daemon-export-ok to\nmean:\n\n    - the presense of empty file means any supported operation\n      not just git-upload-pack is allowed for backward\n      compatibility.\n\n    - otherwise list of allowed git-* server side program.\n\nand be able to extend daemon.c:execute() function to check\nand execute server side programs other than git-upload-pack.  So\nthe answer from me is a mild no.\n"},{"id":"8636","messageId":"46a038f905091514447e13404d@mail.gmail.com","threadId":"1806","inReplyTo":"Pine.LNX.4.58.0509151225410.26803@g5.osdl.org","subject":"Re: git-daemon --inetd","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2005-09-15T21:44:13Z","receivedAt":"2005-09-15T21:44:13Z","isPatch":false,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 9/16/05, Linus Torvalds <torvalds@osdl.org> wrote:\n> Btw, I think --merge-order was cool, but its weaker cousin --topo-order is\n> what is actually _used_. Maybe we should deprecate --merge-order? Right\n> now the only real user is git-archimport, and I think that one too really\n> only wants topo-order too. For example, right now I think git-archimport\n> won't actually work if you build without openssl.\n> \n> Jon? Martin?\n\nI've used merge-order because it made sense... in that it gave me the\nsame history and order that walking the arch history would. Most of\nthe time, anyway, and decided that I'd rather trust GIT over Arch on\nmerging strategies...\n\nIf topo-order is sane, then I'll just change that. Let me run a couple\nof tests with it first ;)\n\nWRT SSL... archimport doesn't care a bit about SSL, and if it does,\nit's a bug. It calls tla to perform all the \"fetch stuff from the Arch\nrepo operations\", because I was too lazy to implement native support\nfor reading the (trivial) repo format over all the transports. And\nwith Arch, access to the remote repos is _fast_ (unlike cvs).\n\ncheers,\n\n\n\nmartin\n"},{"id":"8660","messageId":"7vbr2tsf7o.fsf@assigned-by-dhcp.cox.net","threadId":"1806","inReplyTo":"4329BDD9.4010507@zytor.com","subject":"Re: git-daemon --inetd","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-09-16T06:23:55Z","receivedAt":"2005-09-16T06:23:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"H. Peter Anvin\" <hpa@zytor.com> writes:\n\n> Wrapping it in chroot() would mean having enough things in the chroot \n> environment to support starting up programs, which is ugly.  I'll test \n> the patch when I get a chance.\n\nI'd appreciate a forward of the patch with your acked by if you\nfind it acceptable for kernel.org use.\n\nI'd apply Linus' patch myself and push it out in the proposed\nupdates branch, if you prefer.\n"},{"id":"8666","messageId":"7vbr2tqwl0.fsf@assigned-by-dhcp.cox.net","threadId":"1806","inReplyTo":"Pine.LNX.4.58.0509150829090.26803@g5.osdl.org","subject":"Re: git-daemon --inetd","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-09-16T07:51:39Z","receivedAt":"2005-09-16T07:51:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> Hmm.. That should work fine. You can already just run it that way by just \n> wrapping it in \"chroot\", but if you don't want that for some reason, how \n> about a patch like this?\n\nLater exchanges between you and HPA appeared to me that we would\nneed a chroot environment which has \"enough stuff\" and that this\npatch may not help him very much.  Am I mistaken?\n\nIf not, then...\n\n> +\t\tif (!strncmp(arg, \"--chroot=\", 9)) {\n> +\t\t\tif (chroot(arg+9) < 0)\n> +\t\t\t\tdie(\"unable to chroot to '%s': %s\", arg+9, strerror(errno));\n> +\t\t\tif (chdir(\"/\") < 0)\n> +\t\t\t\tdie(\"unable to chdir to new root\");\n> +\n> +\t\t\tuser = user ? user : \"nobody\";\n> +\t\t\tgroup = group ? group : \"nobody\";\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tif (!strncmp(arg, \"--user=\", 7)) {\n> +\t\t\tuser = arg+7;\n> +\t\t\tcontinue;\n> +\t\t}\n\nI think resolving user and group to numeric before you do\nchroot() might make the setting up of chrooted environment a\nlittle simpler; no need for supporting getpwnam and getgrnam\nthere.  On the other hand it may not matter -- you can always\ngive numeric uid/gid to begin with.\n"},{"id":"8688","messageId":"432AF8A8.6070807@zytor.com","threadId":"1806","inReplyTo":"7vbr2tqwl0.fsf@assigned-by-dhcp.cox.net","subject":"Re: git-daemon --inetd","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-09-16T16:54:00Z","receivedAt":"2005-09-16T16:54:00Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Junio C Hamano wrote:\n> \n> Later exchanges between you and HPA appeared to me that we would\n> need a chroot environment which has \"enough stuff\" and that this\n> patch may not help him very much.  Am I mistaken?\n> \n\nWell, building a chroot environment which supports execing is a bit of a \npain, but it's fully doable.  mount --bind especially makes that quite \nfeasible.  It's just more work.\n\n> I think resolving user and group to numeric before you do\n> chroot() might make the setting up of chrooted environment a\n> little simpler; no need for supporting getpwnam and getgrnam\n> there.  On the other hand it may not matter -- you can always\n> give numeric uid/gid to begin with.\n\nYes, resolve the username first.\n\n\t-hpa\n"},{"id":"8691","messageId":"Pine.LNX.4.58.0509161027460.26803@g5.osdl.org","threadId":"1806","inReplyTo":"7vbr2tqwl0.fsf@assigned-by-dhcp.cox.net","subject":"Re: git-daemon --inetd","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-09-16T17:30:44Z","receivedAt":"2005-09-16T17:30:44Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 16 Sep 2005, Junio C Hamano wrote:\n> \n> Later exchanges between you and HPA appeared to me that we would\n> need a chroot environment which has \"enough stuff\" and that this\n> patch may not help him very much.  Am I mistaken?\n\nNo, I think that's correct.\n\n> I think resolving user and group to numeric before you do\n> chroot() might make the setting up of chrooted environment a\n> little simpler; no need for supporting getpwnam and getgrnam\n> there.\n\nRight you are. Much better.\n\n>  On the other hand it may not matter -- you can always\n> give numeric uid/gid to begin with.\n\nWell, the symbolic names are much nicer and more readable. So it would be \nbetter to do the uid/gid translation early, and change the \"chroot\" thing \nto be done after all that.\n\nIt gets a bit messy.. Easy enough to just save a \"const char *new_root\",\nbut then you have to split up the \"set_user_group()\" to be two functions,\naround the actual chroot(), since the chroot needs to be done while we're\nstill root.\n\nI think we can drop the patch for now. It doesn't buy much.\n\n\t\tLinus\n"},{"id":"8698","messageId":"432B0D97.2030903@zytor.com","threadId":"1806","inReplyTo":"Pine.LNX.4.58.0509161027460.26803@g5.osdl.org","subject":"Re: git-daemon --inetd","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-09-16T18:23:19Z","receivedAt":"2005-09-16T18:23:19Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Linus Torvalds wrote:\n> \n> Well, the symbolic names are much nicer and more readable. So it would be \n> better to do the uid/gid translation early, and change the \"chroot\" thing \n> to be done after all that.\n> \n> It gets a bit messy.. Easy enough to just save a \"const char *new_root\",\n> but then you have to split up the \"set_user_group()\" to be two functions,\n> around the actual chroot(), since the chroot needs to be done while we're\n> still root.\n> \n\nActually, initgroups() and setgroups(), and setgid() for that matter, \ncan be done before the chroot().  The only thing that needs to remain \nuntil the end is setuid().\n\nAt one time I played around in tftp-hpa with trying to get Linux to keep \nonly CAP_SYS_CHROOT around, but I think I gave up on it.  The way Linux \ncapabilities play with the rest of the permission system isn't very \nuseful :(\n\n\t-hpa\n"},{"id":"10462","messageId":"2cfc40320510220645r6e8dc735w32b6ec3633b1d1ff@mail.gmail.com","threadId":"1806","inReplyTo":"46a038f905091514447e13404d@mail.gmail.com","subject":"Re: git-daemon --inetd","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2005-10-22T13:45:31Z","receivedAt":"2005-10-22T13:45:31Z","isPatch":false,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"On 9/16/05, Martin Langhoff <martin.langhoff@gmail.com> wrote:\n> On 9/16/05, Linus Torvalds <torvalds@osdl.org> wrote:\n> > Btw, I think --merge-order was cool, but its weaker cousin --topo-order is\n> > what is actually _used_. Maybe we should deprecate --merge-order? Right\n> > now the only real user is git-archimport, and I think that one too really\n> > only wants topo-order too. For example, right now I think git-archimport\n> > won't actually work if you build without openssl.\n> >\n> > Jon? Martin?\n\nSorry for the long delay in replying - I need to fix my mail filter so\nthat I do see GIT mail that is actually directed to me!\n\nIs the concern with --merge-order the complexity of the logic (and\nhence size of object), the intrusiveness into rev-list.c or the fact\nthat it uses the OPEN_SSL?\n\nI believe I can rework the merge-order algorithm to avoid the need for\nbignum support for OPEN_SSL, though I am not going to get a chance to\ndo that until December at the earliest.\n\nMuch as I hate to see my baby being orphaned, how about we deprecate\n--merge-order for now and arrange things so that users have to\nexplicitly enable it if they want it. If enough people complain about\nthat, then I can produce a bignum-free version in the December\ntimeframe?\n\nRegards,\n\njon.\n"},{"id":"10467","messageId":"Pine.LNX.4.64.0510221404230.10477@g5.osdl.org","threadId":"1806","inReplyTo":"2cfc40320510220645r6e8dc735w32b6ec3633b1d1ff@mail.gmail.com","subject":"Re: git-daemon --inetd","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-10-22T21:05:38Z","receivedAt":"2005-10-22T21:05:38Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 22 Oct 2005, Jon Seymour wrote:\n> \n> Is the concern with --merge-order the complexity of the logic (and\n> hence size of object), the intrusiveness into rev-list.c or the fact\n> that it uses the OPEN_SSL?\n\nSome of all. But mostly just the basic fact being that right now, nobody \ncan really use --merge-order anyway, because if somebody compiles without \nOPEN_SSL, it just won't be there. So it's _practically_ useless.\n\n\t\tLinus\n"}]}