{"thread":{"id":"5331","subject":"[PATCH] Added support for dropping privileges to git-daemon.","startedAt":"2006-08-19T12:27:40Z","lastAt":"2006-08-23T16:45:18Z","messageCount":13,"participants":["Tilman Sauerbeck","Marco Costalba","Johannes Schindelin","Mitchell Blank Jr","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"25610","messageId":"1155990772.6591@hammerfest","threadId":"5331","inReplyTo":null,"subject":"[PATCH] Added support for dropping privileges to git-daemon.","fromName":"Tilman Sauerbeck","fromEmail":"tilman@code-monkey.de","sentAt":"2006-08-19T12:27:40Z","receivedAt":"2006-08-19T12:27:40Z","isPatch":true,"sender":{"key":"tilman@code-monkey.de","avatar":null},"body":"Signed-off-by: Tilman Sauerbeck \n---\n\n I'm not sure how useful this is. I'd like to start git-daemon as root,\n so it can store it's PID in /var/run, but I don't want it to keep root\n privileges. My git repos are world readable, so for serving them, root\n privileges aren't needed at all.\n\n What do you think?\n\n\n Documentation/git-daemon.txt | 8 +++++++-\n daemon.c | 39 ++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 45 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\nindex 0f7d274..a8d75d9 100644\n--- a/Documentation/git-daemon.txt\n+++ b/Documentation/git-daemon.txt\n@@ -11,7 +11,8 @@ SYNOPSIS\n 'git-daemon' [--verbose] [--syslog] [--inetd | --port=n] [--export-all]\n [--timeout=n] [--init-timeout=n] [--strict-paths]\n [--base-path=path] [--user-path | --user-path=path]\n-\t [--reuseaddr] [--detach] [--pid-file=file] [directory...]\n+\t [--reuseaddr] [--detach] [--pid-file=file]\n+\t [--user=u] [--group=g] [directory...]\n \n DESCRIPTION\n -----------\n@@ -93,6 +94,11 @@ OPTIONS\n --pid-file=file::\n \tSave the process id in 'file'.\n \n+--user=u::\n+--group=g::\n+\tIf both options are given, `git-daemon` will change it's uid and gid to\n+\tthe ones of 'u' and 'g' before entering the server loop.\n+\n ::\n \tA directory to add to the whitelist of allowed directories. Unless\n \t--strict-paths is specified this will also include subdirectories\ndiff --git a/daemon.c b/daemon.c\nindex 012936f..78658c1 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -7,6 +7,8 @@ #include \n #include \n #include \n #include \n+#include \n+#include \n #include \"pkt-line.h\"\n #include \"cache.h\"\n #include \"exec_cmd.h\"\n@@ -14,12 +16,15 @@ #include \"exec_cmd.h\"\n static int log_syslog;\n static int verbose;\n static int reuseaddr;\n+static const char *user;\n+static const char *group;\n \n static const char daemon_usage[] =\n \"git-daemon [--verbose] [--syslog] [--inetd | --port=n] [--export-all]\\n\"\n \" [--timeout=n] [--init-timeout=n] [--strict-paths]\\n\"\n \" [--base-path=path] [--user-path | --user-path=path]\\n\"\n-\" [--reuseaddr] [--detach] [--pid-file=file] [directory...]\";\n+\" [--reuseaddr] [--detach] [--pid-file=file]\\n\"\n+\" [--user=u] [--group=g] [directory...]\";\n \n /* List of acceptable pathname prefixes */\n static char **ok_paths;\n@@ -701,6 +706,24 @@ static void store_pid(const char *path)\n \tfclose(f);\n }\n \n+static void drop_privileges()\n+{\n+\tstruct passwd *p;\n+\tstruct group *g;\n+\n+\tp = getpwnam (user);\n+\tif (!p)\n+\t\tdie(\"user not found - %s\", user);\n+\n+\tg = getgrnam (group);\n+\tif (!g)\n+\t\tdie(\"group not found - %s\", group);\n+\n+\tif (initgroups (p->pw_name, g->gr_gid) || setgid (g->gr_gid) ||\n+\t setuid (p->pw_uid))\n+\t\tdie(\"cannot drop privileges\");\n+}\n+\n static int serve(int port)\n {\n \tint socknum, *socklist;\n@@ -709,6 +732,9 @@ static int serve(int port)\n \tif (socknum == 0)\n \t\tdie(\"unable to allocate any listen sockets on port %u\", port);\n \n+\tif (user && group)\n+\t\tdrop_privileges();\n+\n \treturn service_loop(socknum, socklist);\n }\n \n@@ -791,6 +817,14 @@ int main(int argc, char **argv)\n \t\t\tlog_syslog = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strncmp(arg, \"--user=\", 7)) {\n+\t\t\tuser = arg + 7;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strncmp(arg, \"--group=\", 8)) {\n+\t\t\tgroup = arg + 8;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!strcmp(arg, \"--\")) {\n \t\t\tok_paths = &argv[i+1];\n \t\t\tbreak;\n@@ -802,6 +836,9 @@ int main(int argc, char **argv)\n \t\tusage(daemon_usage);\n \t}\n \n+\tif (!user ^ !group)\n+\t\tdie(\"either set both user and group or none of them\");\n+\n \tif (log_syslog) {\n \t\topenlog(\"git-daemon\", 0, LOG_DAEMON);\n \t\tset_die_routine(daemon_die);\n-- \n1.4.2\n"},{"id":"25644","messageId":"1156079371.28098@hammerfest","threadId":"5331","inReplyTo":"20060819172533.GB64962@gaz.sfgoth.com","subject":"[PATCH] Added support for dropping privileges to git-daemon.","fromName":"Tilman Sauerbeck","fromEmail":"tilman@code-monkey.de","sentAt":"2006-08-19T12:27:40Z","receivedAt":"2006-08-19T12:27:40Z","isPatch":true,"sender":{"key":"tilman@code-monkey.de","avatar":null},"body":"Signed-off-by: Tilman Sauerbeck \n---\n\n My idea was to keep the code simple :)\n\n Anyway, this patch has the code you proposed.\n\n Documentation/git-daemon.txt | 11 +++++++++-\n daemon.c | 45 +++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 54 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\nindex 0f7d274..3c1bc85 100644\n--- a/Documentation/git-daemon.txt\n+++ b/Documentation/git-daemon.txt\n@@ -11,7 +11,8 @@ SYNOPSIS\n 'git-daemon' [--verbose] [--syslog] [--inetd | --port=n] [--export-all]\n [--timeout=n] [--init-timeout=n] [--strict-paths]\n [--base-path=path] [--user-path | --user-path=path]\n-\t [--reuseaddr] [--detach] [--pid-file=file] [directory...]\n+\t [--reuseaddr] [--detach] [--pid-file=file]\n+\t [--user=u [--group=g]] [directory...]\n \n DESCRIPTION\n -----------\n@@ -93,6 +94,14 @@ OPTIONS\n --pid-file=file::\n \tSave the process id in 'file'.\n \n+--user=u::\n+--group=g::\n+\tThese two options may be used to make `git-daemon` change its uid and\n+\tgid\tbefore entering the server loop.\n+\tThe uid that's used is the one of 'u'. If `group` is specified,\n+\tthe gid is set to the one of 'g', otherwise, the default gid\n+\tof 'u' is used.\n+\n ::\n \tA directory to add to the whitelist of allowed directories. Unless\n \t--strict-paths is specified this will also include subdirectories\ndiff --git a/daemon.c b/daemon.c\nindex 012936f..70be10f 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -7,6 +7,8 @@ #include \n #include \n #include \n #include \n+#include \n+#include \n #include \"pkt-line.h\"\n #include \"cache.h\"\n #include \"exec_cmd.h\"\n@@ -14,12 +16,15 @@ #include \"exec_cmd.h\"\n static int log_syslog;\n static int verbose;\n static int reuseaddr;\n+static const char *user;\n+static const char *group;\n \n static const char daemon_usage[] =\n \"git-daemon [--verbose] [--syslog] [--inetd | --port=n] [--export-all]\\n\"\n \" [--timeout=n] [--init-timeout=n] [--strict-paths]\\n\"\n \" [--base-path=path] [--user-path | --user-path=path]\\n\"\n-\" [--reuseaddr] [--detach] [--pid-file=file] [directory...]\";\n+\" [--reuseaddr] [--detach] [--pid-file=file]\\n\"\n+\" [--user=u] [--group=g] [directory...]\";\n \n /* List of acceptable pathname prefixes */\n static char **ok_paths;\n@@ -701,6 +706,30 @@ static void store_pid(const char *path)\n \tfclose(f);\n }\n \n+static void drop_privileges()\n+{\n+\tstruct passwd *p;\n+\tstruct group *g;\n+\tgid_t gid;\n+\n+\tp = getpwnam (user);\n+\tif (!p)\n+\t\tdie(\"user not found - %s\", user);\n+\n+\tif (!group)\n+\t\tgid = p->pw_gid;\n+\telse {\n+\t\tg = getgrnam (group);\n+\t\tif (!g)\n+\t\t\tdie(\"group not found - %s\", group);\n+\n+\t\tgid = g->gr_gid;\n+\t}\n+\n+\tif (initgroups (p->pw_name, gid) || setgid (gid) || setuid (p->pw_uid))\n+\t\tdie(\"cannot drop privileges\");\n+}\n+\n static int serve(int port)\n {\n \tint socknum, *socklist;\n@@ -709,6 +738,9 @@ static int serve(int port)\n \tif (socknum == 0)\n \t\tdie(\"unable to allocate any listen sockets on port %u\", port);\n \n+\tif (user)\n+\t\tdrop_privileges();\n+\n \treturn service_loop(socknum, socklist);\n }\n \n@@ -791,6 +823,14 @@ int main(int argc, char **argv)\n \t\t\tlog_syslog = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strncmp(arg, \"--user=\", 7)) {\n+\t\t\tuser = arg + 7;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strncmp(arg, \"--group=\", 8)) {\n+\t\t\tgroup = arg + 8;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!strcmp(arg, \"--\")) {\n \t\t\tok_paths = &argv[i+1];\n \t\t\tbreak;\n@@ -802,6 +842,9 @@ int main(int argc, char **argv)\n \t\tusage(daemon_usage);\n \t}\n \n+\tif (group && !user)\n+\t\tdie(\"--group supplied without --user\");\n+\n \tif (log_syslog) {\n \t\topenlog(\"git-daemon\", 0, LOG_DAEMON);\n \t\tset_die_routine(daemon_die);\n-- \n1.4.2\n"},{"id":"25611","messageId":"e5bfff550608190623j58de8c1cn6a9304249ee1ecb8@mail.gmail.com","threadId":"5331","inReplyTo":"1155990772.6591@hammerfest","subject":"Re: [PATCH] Added support for dropping privileges to git-daemon.","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2006-08-19T13:23:03Z","receivedAt":"2006-08-19T13:23:03Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":">\n> + if (!user ^ !group)\n> + die(\"either set both user and group or none of them\");\n> +\n>\n\nJust a question. Why simply not\n\n if (user ^ group)\n"},{"id":"25612","messageId":"20060819132922.GA6644@code-monkey.de","threadId":"5331","inReplyTo":"e5bfff550608190623j58de8c1cn6a9304249ee1ecb8@mail.gmail.com","subject":"Re: [PATCH] Added support for dropping privileges to git-daemon.","fromName":"Tilman Sauerbeck","fromEmail":"tilman@code-monkey.de","sentAt":"2006-08-19T13:29:24Z","receivedAt":"2006-08-19T13:29:24Z","isPatch":true,"sender":{"key":"tilman@code-monkey.de","avatar":null},"body":"Marco Costalba [2006-08-19 15:23]:\n> >\n> >+ if (!user ^ !group)\n> >+ die(\"either set both user and group or none of them\");\n> >+\n> >\n> \n> Just a question. Why simply not\n> \n> if (user ^ group)\n\nBecause gcc doesn't like that:\n error: invalid operands to binary ^\n\nRegards,\nTilman\n\n-- \nA: Because it messes up the order in which people normally read text.\nQ: Why is top-posting such a bad thing?\nA: Top-posting.\nQ: What is the most annoying thing on usenet and in e-mail?\n"},{"id":"25613","messageId":"Pine.LNX.4.63.0608191529300.28360@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"5331","inReplyTo":"1155990772.6591@hammerfest","subject":"Re: [PATCH] Added support for dropping privileges to git-daemon.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-08-19T13:32:10Z","receivedAt":"2006-08-19T13:32:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 19 Aug 2006, Tilman Sauerbeck wrote:\n\n> What do you think?\n\nI think it is a good addition. Note that most people will probably use \ninetd instead, though.\n\n> +\t [--user=u] [--group=g] [directory...]\n\nSince you enforce both --user and --group, this should read\n\n\t [--user=u --group=g]\n\nCiao,\nDscho\n"},{"id":"25617","messageId":"e5bfff550608190819i3cade548g28b2c95fab172a49@mail.gmail.com","threadId":"5331","inReplyTo":"20060819132922.GA6644@code-monkey.de","subject":"Re: [PATCH] Added support for dropping privileges to git-daemon.","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2006-08-19T15:19:30Z","receivedAt":"2006-08-19T15:19:30Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 8/19/06, Tilman Sauerbeck wrote:\n> Marco Costalba [2006-08-19 15:23]:\n> > >\n> > >+ if (!user ^ !group)\n> > >+ die(\"either set both user and group or none of them\");\n> > >+\n> > >\n> >\n> > Just a question. Why simply not\n> >\n> > if (user ^ group)\n>\n> Because gcc doesn't like that:\n> error: invalid operands to binary ^\n>\n\nInteresting!\n\nIndeed gcc it's right, operator ^ it's a bitwise operator, not a\nlogical one. So operands must be booleans (or numerical types in case\nof C).\n\nThe trick is that operator '!' performs an implicit conversion to\nintegral on the 'user' and 'group' pointers.\n\nBTW the following (very ugly) works.\n\n if ((int)user ^ (int)group)\n"},{"id":"25618","messageId":"e5bfff550608190822o6d99c390he15612aa3f8afd14@mail.gmail.com","threadId":"5331","inReplyTo":"e5bfff550608190819i3cade548g28b2c95fab172a49@mail.gmail.com","subject":"Re: [PATCH] Added support for dropping privileges to git-daemon.","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2006-08-19T15:22:08Z","receivedAt":"2006-08-19T15:22:08Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"> logical one. So operands must be booleans (or numerical types in case\n> of C).\n\nOperands must be numerical types of course, not booleans!!\n"},{"id":"25621","messageId":"20060819171534.GA64962@gaz.sfgoth.com","threadId":"5331","inReplyTo":"e5bfff550608190819i3cade548g28b2c95fab172a49@mail.gmail.com","subject":"Re: [PATCH] Added support for dropping privileges to git-daemon.","fromName":"Mitchell Blank Jr","fromEmail":"mitch@sfgoth.com","sentAt":"2006-08-19T17:15:34Z","receivedAt":"2006-08-19T17:15:34Z","isPatch":true,"sender":{"key":"mitch@sfgoth.com","avatar":null},"body":"Marco Costalba wrote:\n> >> >+ if (!user ^ !group)\n> >> >+ die(\"either set both user and group or none of them\");\n> \n> BTW the following (very ugly) works.\n> \n> if ((int)user ^ (int)group)\n\nNo it doesn't. Besides being a dangerous cast (no guarantee that a pointer\nwill fit in an \"int\") your code is basically just a fancy way of saying\n\n\tif (user != group)\n\nWhich is definitely NOT what is intended. \"user\" and \"group\" are pointers --\nunless they're both NULL we expect them to have different values. The\noriginal code is equivalent to:\n\n\tif ((user == NULL) != (group == NULL))\n\nwhich is what is actually intended.\n\n-Mitch\n"},{"id":"25622","messageId":"20060819172533.GB64962@gaz.sfgoth.com","threadId":"5331","inReplyTo":"1155990772.6591@hammerfest","subject":"Re: [PATCH] Added support for dropping privileges to git-daemon.","fromName":"Mitchell Blank Jr","fromEmail":"mitch@sfgoth.com","sentAt":"2006-08-19T17:25:33Z","receivedAt":"2006-08-19T17:25:33Z","isPatch":true,"sender":{"key":"mitch@sfgoth.com","avatar":null},"body":"Tilman Sauerbeck wrote:\n> +\tif (user && group)\n> +\t\tdrop_privileges();\n\nIt seems \"if (user)\" would be sufficient here.\n\n> +\tif (!user ^ !group)\n> +\t\tdie(\"either set both user and group or none of them\");\n\nFor simplicities sake I'd actually suggest allowing group==NULL. So\nchange this test to be\n\n\tif (group && !user)\n\t\tdie(\"--group supplied without --user\");\n\nThen in drop_privileges() do something like:\n\n\tstruct passwd *p;\n\tgid_t gid;\n\n\tp = getpwnam(user);\n\tif (!p)\n\t\tdie(\"user not found - %s\", user);\n\tif (group == NULL)\n\t\tgid = p->pw_gid;\n\telse {\n\t\tstruct group *g = getgrnam(group);\n\t\tif (!g)\n\t\t\tdie(\"group not found - %s\", group);\n\t\tgid = g->gr_gid;\n\t}\n\nSince usually you want to use the same gid that is normally associated\nwith that pid, this just makes things a little easier on the user\n\n-Mitch\n"},{"id":"25725","messageId":"7vhd05s2b5.fsf@assigned-by-dhcp.cox.net","threadId":"5331","inReplyTo":"1156079371.28098@hammerfest","subject":"Re: [PATCH] Added support for dropping privileges to git-daemon.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-22T06:38:06Z","receivedAt":"2006-08-22T06:38:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tilman Sauerbeck writes:\n\n> +\t [--reuseaddr] [--detach] [--pid-file=file]\n> +\t [--user=u [--group=g]] [directory...]\n> \n> DESCRIPTION\n> -----------\n> @@ -93,6 +94,14 @@ OPTIONS\n> --pid-file=file::\n> \tSave the process id in 'file'.\n> \n> +--user=u::\n> +--group=g::\n> +\tThese two options may be used to make `git-daemon` change its uid and\n> +\tgid\tbefore entering the server loop.\n> +\tThe uid that's used is the one of 'u'. If `group` is specified,\n> +\tthe gid is set to the one of 'g', otherwise, the default gid\n> +\tof 'u' is used.\n> +\n> ::\n> \tA directory to add to the whitelist of allowed directories. Unless\n> \t--strict-paths is specified this will also include subdirectories\n\nI'd prefer to spell and out, and clarify the description\nthat the parameter parsing code would do getpwent() for the\nuser, and the user does not have to supply numeric user and\ngroup ids, and what happens if numeric ids are given.\n\nHow well does this interact with --inetd (inetd_mode)? Do we\nwant to have some notes in the documentation on that topic? \n\n> diff --git a/daemon.c b/daemon.c\n> index 012936f..70be10f 100644\n> --- a/daemon.c\n> +++ b/daemon.c\n> @@ -7,6 +7,8 @@ #include \n> #include \n> #include \n> #include \n> +#include \n> +#include \n> #include \"pkt-line.h\"\n> #include \"cache.h\"\n> #include \"exec_cmd.h\"\n> @@ -14,12 +16,15 @@ #include \"exec_cmd.h\"\n> static int log_syslog;\n> static int verbose;\n> static int reuseaddr;\n> +static const char *user;\n> +static const char *group;\n\nDo these have to be file-level global?\n\nDo we want to do getpwnam/getgrnam lookup in drop_privileges()?\n\"Gaaaah, no such username/groupname!\" would be needed anyway,\nand I have a feeling that it might be better done once\nimmediately after argument parsing. Then you can pass whatever\nis needed for initgroups/setgid/setuid down as parameters to\nserve() and drop_privileges().\n\nDo we want to use initgroups(), which is _BSD_SOURCE? I guess\nit is perhaps Ok.\n\n> +static void drop_privileges()\n> +{\n> +\tstruct passwd *p;\n> +\tstruct group *g;\n> +\tgid_t gid;\n> +\n> +\tp = getpwnam (user);\n\nNo space between function name and open parenthesis, please.\n\n> @@ -709,6 +738,9 @@ static int serve(int port)\n> \tif (socknum == 0)\n> \t\tdie(\"unable to allocate any listen sockets on port %u\", port);\n> \n> +\tif (user)\n> +\t\tdrop_privileges();\n> +\n> \treturn service_loop(socknum, socklist);\n> }\n\nMakes one wonder why it checks only user and not group, and then\nmakes one realize that the argument parsing code enforces that\ngroup is never supplied without user, which leaves funny taste\nin the mouth, but that is Ok, I guess.\n"},{"id":"25755","messageId":"1156268432.16120@hammerfest","threadId":"5331","inReplyTo":"7vhd05s2b5.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] Added support for dropping privileges to git-daemon.","fromName":"Tilman Sauerbeck","fromEmail":"tilman@code-monkey.de","sentAt":"2006-08-22T17:37:41Z","receivedAt":"2006-08-22T17:37:41Z","isPatch":true,"sender":{"key":"tilman@code-monkey.de","avatar":null},"body":"Signed-off-by: Tilman Sauerbeck \n---\n\n This one got the changes you proposed.\n\n Not sure whether the documentation is good enough, I'm not that good at\n writing tech docs in english :(\n\n\n Documentation/git-daemon.txt | 18 ++++++++++++++++\n daemon.c | 46 ++++++++++++++++++++++++++++++++++++++----\n 2 files changed, 59 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\nindex 0f7d274..b591895 100644\n--- a/Documentation/git-daemon.txt\n+++ b/Documentation/git-daemon.txt\n@@ -11,7 +11,8 @@ SYNOPSIS\n 'git-daemon' [--verbose] [--syslog] [--inetd | --port=n] [--export-all]\n [--timeout=n] [--init-timeout=n] [--strict-paths]\n [--base-path=path] [--user-path | --user-path=path]\n-\t [--reuseaddr] [--detach] [--pid-file=file] [directory...]\n+\t [--reuseaddr] [--detach] [--pid-file=file]\n+\t [--user=user [--group=group]] [directory...]\n \n DESCRIPTION\n -----------\n@@ -93,6 +94,21 @@ OPTIONS\n --pid-file=file::\n \tSave the process id in 'file'.\n \n+--user=user::\n+--group=group::\n+\tThese two options may be used to make `git-daemon` change its uid and\n+\tgid\tbefore entering the server loop.\n+\tThe uid that's used is the one of 'user'. If `group` is specified,\n+\tthe gid is set to the one of 'group', otherwise, the default gid\n+\tof 'user' is used.\n+\n+\tBoth `group` and `user` need to be passed as the name of the resp of\n+\tthe group, ie you'll get unexpected results if you pass an uid/a gid.\n+\n+\tNote that you probably don't want to use these options if you run\n+\tgit-daemon in inetd mode, since inetd can do the privilege dropping\n+\tfor you.\n+\n ::\n \tA directory to add to the whitelist of allowed directories. Unless\n \t--strict-paths is specified this will also include subdirectories\ndiff --git a/daemon.c b/daemon.c\nindex 012936f..4e94210 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -7,6 +7,8 @@ #include \n #include \n #include \n #include \n+#include \n+#include \n #include \"pkt-line.h\"\n #include \"cache.h\"\n #include \"exec_cmd.h\"\n@@ -19,7 +21,8 @@ static const char daemon_usage[] =\n \"git-daemon [--verbose] [--syslog] [--inetd | --port=n] [--export-all]\\n\"\n \" [--timeout=n] [--init-timeout=n] [--strict-paths]\\n\"\n \" [--base-path=path] [--user-path | --user-path=path]\\n\"\n-\" [--reuseaddr] [--detach] [--pid-file=file] [directory...]\";\n+\" [--reuseaddr] [--detach] [--pid-file=file]\\n\"\n+\" [--user=user [[--group=group]] [directory...]\";\n \n /* List of acceptable pathname prefixes */\n static char **ok_paths;\n@@ -701,7 +704,7 @@ static void store_pid(const char *path)\n \tfclose(f);\n }\n \n-static int serve(int port)\n+static int serve(int port, struct passwd *pass, gid_t gid)\n {\n \tint socknum, *socklist;\n \n@@ -709,6 +712,11 @@ static int serve(int port)\n \tif (socknum == 0)\n \t\tdie(\"unable to allocate any listen sockets on port %u\", port);\n \n+\tif (pass && gid &&\n+\t (initgroups(pass->pw_name, gid) || setgid (gid) ||\n+\t setuid(pass->pw_uid)))\n+\t\tdie(\"cannot drop privileges\");\n+\n \treturn service_loop(socknum, socklist);\n }\n \n@@ -716,8 +724,11 @@ int main(int argc, char **argv)\n {\n \tint port = DEFAULT_GIT_PORT;\n \tint inetd_mode = 0;\n-\tconst char *pid_file = NULL;\n+\tconst char *pid_file = NULL, *user_name = NULL, *group_name = NULL;\n \tint detach = 0;\n+\tstruct passwd *pass = NULL;\n+\tstruct group *group;\n+\tgid_t gid = 0;\n \tint i;\n \n \t/* Without this we cannot rely on waitpid() to tell\n@@ -791,6 +802,14 @@ int main(int argc, char **argv)\n \t\t\tlog_syslog = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strncmp(arg, \"--user=\", 7)) {\n+\t\t\tuser_name = arg + 7;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strncmp(arg, \"--group=\", 8)) {\n+\t\t\tgroup_name = arg + 8;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!strcmp(arg, \"--\")) {\n \t\t\tok_paths = &argv[i+1];\n \t\t\tbreak;\n@@ -802,6 +821,25 @@ int main(int argc, char **argv)\n \t\tusage(daemon_usage);\n \t}\n \n+\tif (group_name && !user_name)\n+\t\tdie(\"--group supplied without --user\");\n+\n+\tif (user_name) {\n+\t\tpass = getpwnam(user_name);\n+\t\tif (!pass)\n+\t\t\tdie(\"user not found - %s\", user_name);\n+\n+\t\tif (!group_name)\n+\t\t\tgid = pass->pw_gid;\n+\t\telse {\n+\t\t\tgroup = getgrnam(group_name);\n+\t\t\tif (!group)\n+\t\t\t\tdie(\"group not found - %s\", group_name);\n+\n+\t\t\tgid = group->gr_gid;\n+\t\t}\n+\t}\n+\n \tif (log_syslog) {\n \t\topenlog(\"git-daemon\", 0, LOG_DAEMON);\n \t\tset_die_routine(daemon_die);\n@@ -831,5 +869,5 @@ int main(int argc, char **argv)\n \tif (pid_file)\n \t\tstore_pid(pid_file);\n \n-\treturn serve(port);\n+\treturn serve(port, pass, gid);\n }\n-- \n1.4.2\n"},{"id":"25773","messageId":"7virkkl4ph.fsf@assigned-by-dhcp.cox.net","threadId":"5331","inReplyTo":"1156268432.16120@hammerfest","subject":"Re: [PATCH] Added support for dropping privileges to git-daemon.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-22T23:40:26Z","receivedAt":"2006-08-22T23:40:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tilman Sauerbeck writes:\n\n> @@ -93,6 +94,21 @@ OPTIONS\n> --pid-file=file::\n> \tSave the process id in 'file'.\n> \n> +--user=user::\n> +--group=group::\n\nProbably\n\n\t--user=user, --group=group::\n\nAlso check for asciidoc formatting please; it's rather picky.\n\n> +\tThese two options may be used to make `git-daemon` change its uid and\n> +\tgid\tbefore entering the server loop.\n> +\tThe uid that's used is the one of 'user'. If `group` is specified,\n> +\tthe gid is set to the one of 'group', otherwise, the default gid\n> +\tof 'user' is used.\n\nFunny whitespaces all over the place...\n\nWhat is the pw_gid stored in struct passwd for the user?\ngetgroups(2) gives supplementary group IDs, so perhaps it is\ncalled primary group ID?\n\n> +\tBoth `group` and `user` need to be passed as the name of the resp of\n> +\tthe group, ie you'll get unexpected results if you pass an uid/a gid.\n\nGaah, but that is probably OK. I'd explicitly say they are\nalways interpreted as name and never numeric. An alternative\nwould be to be nice and when getpwnam() and/or getgrnam()\nreturns NULL try to interpret them as numeric, which might help\na small dedicated server installation that does not have any\n/etc/passwd or /etc/group file ;-) but I do not think that would\nbe worth the confusion.\n\n> +\tNote that you probably don't want to use these options if you run\n> +\tgit-daemon in inetd mode, since inetd can do the privilege dropping\n> +\tfor you.\n\nGaah again. These options do not have any effect (other than\nsanity checking) on the inetd_mode codepath, so instead of\nsaying this in the documentation I would suggest specifying\nthese options an error under --inetd.\n\nSomething like this on top of your patch perhaps.\n\ndiff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\nindex 8f850fa..17619a3 100644\n--- a/Documentation/git-daemon.txt\n+++ b/Documentation/git-daemon.txt\n@@ -94,20 +94,16 @@ OPTIONS\n --pid-file=file::\n \tSave the process id in 'file'.\n \n---user=user::\n---group=group::\n-\tThese two options may be used to make `git-daemon` change its uid and\n-\tgid\tbefore entering the server loop.\n-\tThe uid that's used is the one of 'user'. If `group` is specified,\n-\tthe gid is set to the one of 'group', otherwise, the default gid\n-\tof 'user' is used.\n-\n-\tBoth `group` and `user` need to be passed as the name of the resp of\n-\tthe group, ie you'll get unexpected results if you pass an uid/a gid.\n-\n-\tNote that you probably don't want to use these options if you run\n-\tgit-daemon in inetd mode, since inetd can do the privilege dropping\n-\tfor you.\n+--user=user, --group=group::\n+\tChange daemon's uid and gid before entering the service loop.\n+\tWhen only `--user` is given without `--group`, the\n+\tprimary group ID for the user is used. The values of\n+\tthe option are given to `getpwnam(3)` and `getgrnam(3)`\n+\tand numeric IDs are not supported.\n++\n+Giving these options is an error when used with `--inetd`; use\n+the facility of inet daemon to achieve the same before spawning\n+`git-daemon` if needed.\n \n ::\n \tA directory to add to the whitelist of allowed directories. Unless\ndiff --git a/daemon.c b/daemon.c\nindex 4e94210..dd3915a 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -821,6 +821,9 @@ int main(int argc, char **argv)\n \t\tusage(daemon_usage);\n \t}\n \n+\tif (inetd_mode && (group_name || user_name))\n+\t\tdie(\"--user and --group are incompatible with --inetd\");\n+\n \tif (group_name && !user_name)\n \t\tdie(\"--group supplied without --user\");\n \n"},{"id":"25811","messageId":"20060823164517.GA1066@code-monkey.de","threadId":"5331","inReplyTo":"7virkkl4ph.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Added support for dropping privileges to git-daemon.","fromName":"Tilman Sauerbeck","fromEmail":"tilman@code-monkey.de","sentAt":"2006-08-23T16:45:18Z","receivedAt":"2006-08-23T16:45:18Z","isPatch":true,"sender":{"key":"tilman@code-monkey.de","avatar":null},"body":"Junio C Hamano [2006-08-22 16:40]:\n> Tilman Sauerbeck writes:\n> \n> > +\tThese two options may be used to make `git-daemon` change its uid and\n> > +\tgid\tbefore entering the server loop.\n> > +\tThe uid that's used is the one of 'user'. If `group` is specified,\n> > +\tthe gid is set to the one of 'group', otherwise, the default gid\n> > +\tof 'user' is used.\n> \n> Funny whitespaces all over the place...\n\nThere's exactly _one_ stray \\t in the middle of a line. Not quite what\nI'd call \"all over the place' ;)\nI should probably tell vim to hilight space errors in txt files, too.\n\n> Gaah again. These options do not have any effect (other than\n> sanity checking) on the inetd_mode codepath, so instead of\n> saying this in the documentation I would suggest specifying\n> these options an error under --inetd.\n\nGood point.\n\nRegards,\nTilman\n\n-- \nA: Because it messes up the order in which people normally read text.\nQ: Why is top-posting such a bad thing?\nA: Top-posting.\nQ: What is the most annoying thing on usenet and in e-mail?\n"}]}