Added support for dropping privileges to git-daemon.

daemon.c static int serve(int port) · line 736

It seems "if (user)" would be sufficient here.

daemon.c int main(int argc, char **argv) · line 840

For simplicities sake I'd actually suggest allowing group==NULL. So change this test to be

	if (group && !user)
		die("--group supplied without --user");

Then in drop_privileges() do something like:

	struct passwd *p;
	gid_t gid;

	p = getpwnam(user);
	if (!p)
		die("user not found - %s", user);
	if (group == NULL)
		gid = p->pw_gid;
	else {
		struct group *g = getgrnam(group);
		if (!g)
			die("group not found - %s", group);
		gid = g->gr_gid;
	}

Since usually you want to use the same gid that is normally associated with that pid, this just makes things a little easier on the user

-Mitch

Documentation/git-daemon.txt OPTIONS · line 107

I'd prefer to spell <u> and <g> out, and clarify the description that the parameter parsing code would do getpwent() for the user, and the user does not have to supply numeric user and group ids, and what happens if numeric ids are given.

How well does this interact with --inetd (inetd_mode)? Do we want to have some notes in the documentation on that topic?

daemon.c #include "exec_cmd.h" · line 20

Do these have to be file-level global?

Do we want to do getpwnam/getgrnam lookup in drop_privileges()? "Gaaaah, no such username/groupname!" would be needed anyway, and I have a feeling that it might be better done once immediately after argument parsing. Then you can pass whatever is needed for initgroups/setgid/setuid down as parameters to serve() and drop_privileges().

Do we want to use initgroups(), which is _BSD_SOURCE? I guess it is perhaps Ok.

daemon.c static void store_pid(const char *path) · line 715

No space between function name and open parenthesis, please.

daemon.c static int serve(int port) · line 745

Makes one wonder why it checks only user and not group, and then makes one realize that the argument parsing code enforces that group is never supplied without user, which leaves funny taste in the mouth, but that is Ok, I guess.

Review OPTIONS · line 98

Probably

	--user=user, --group=group::

Also check for asciidoc formatting please; it's rather picky.

Review OPTIONS · line 103

Funny whitespaces all over the place...

What is the pw_gid stored in struct passwd for the user? getgroups(2) gives supplementary group IDs, so perhaps it is called primary group ID?

Review OPTIONS · line 105

Gaah, but that is probably OK. I'd explicitly say they are always interpreted as name and never numeric. An alternative would be to be nice and when getpwnam() and/or getgrnam() returns NULL try to interpret them as numeric, which might help a small dedicated server installation that does not have any /etc/passwd or /etc/group file ;-) but I do not think that would be worth the confusion.

Review OPTIONS · line 108

Gaah again. These options do not have any effect (other than sanity checking) on the inetd_mode codepath, so instead of saying this in the documentation I would suggest specifying these options an error under --inetd.

Something like this on top of your patch perhaps.

index 8f850fa..17619a3 100644
@@ -94,20 +94,16 @@ OPTIONS
 --pid-file=file::
 	Save the process id in 'file'.
 
---user=user::
---group=group::
-	These two options may be used to make `git-daemon` change its uid and
-	gid	before entering the server loop.
-	The uid that's used is the one of 'user'. If `group` is specified,
-	the gid is set to the one of 'group', otherwise, the default gid
-	of 'user' is used.
-
-	Both `group` and `user` need to be passed as the name of the resp of
-	the group, ie you'll get unexpected results if you pass an uid/a gid.
-
-	Note that you probably don't want to use these options if you run
-	git-daemon in inetd mode, since inetd can do the privilege dropping
-	for you.
+--user=user, --group=group::
+	Change daemon's uid and gid before entering the service loop.
+	When only `--user` is given without `--group`, the
+	primary group ID for the user is used.  The values of
+	the option are given to `getpwnam(3)` and `getgrnam(3)`
+	and numeric IDs are not supported.
++
+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.
 
 <directory>::
 	A directory to add to the whitelist of allowed directories. Unless

index 4e94210..dd3915a 100644
@@ -821,6 +821,9 @@ int main(int argc, char **argv)
 		usage(daemon_usage);
 	}
 
+	if (inetd_mode && (group_name || user_name))
+		die("--user and --group are incompatible with --inetd");
+
 	if (group_name && !user_name)
 		die("--group supplied without --user");
 

Documentation/git-daemon.txt SYNOPSIS · line 15

Since you enforce both --user and --group, this should read

	 [--user=u --group=g]

Ciao,
Dscho

daemon.c int main(int argc, char **argv) · line 841

Just a question. Why simply not

if (user ^ group)